Add flag to identify block swap devices
in [Kernel]
|
Prev: [git pull] more PCMCIA bugfixes for 2.6.34-rc6
Next: [PATCH] s3c-fb: added patch series for s5pv210 and some features.
From: jassi brar on 7 May 2010 04:10 On Fri, May 7, 2010 at 4:25 PM, Nitin Gupta <ngupta(a)vflare.org> wrote: > Added SWP_BLKDEV flag to distinguish block and regular file backed > swap devices. We could also check if a swap is entire block device, > rather than a file, by: > S_ISBLK(swap_info_struct->swap_file->f_mapping->host->i_mode) > but, I think, simply checking this flag is more convenient. This might make it convenient for now but is likely to increase complexity and redundancy. Why not define a macro/inline to figure that out? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Nitin Gupta on 7 May 2010 04:30 On 05/07/2010 01:33 PM, jassi brar wrote: > On Fri, May 7, 2010 at 4:25 PM, Nitin Gupta <ngupta(a)vflare.org> wrote: >> Added SWP_BLKDEV flag to distinguish block and regular file backed >> swap devices. We could also check if a swap is entire block device, >> rather than a file, by: >> S_ISBLK(swap_info_struct->swap_file->f_mapping->host->i_mode) >> but, I think, simply checking this flag is more convenient. > This might make it convenient for now but is likely to increase complexity and > redundancy. Why not define a macro/inline to figure that out? > Accessing such long pointer chain is maybe not good thing to do for every swap_entry_free() call? Simple checking a flag is perhaps slightly faster? I also can't see how this flag can later increase complexity compared to creating new macro for this check. Thanks, Nitin -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: jassi brar on 7 May 2010 05:00 On Fri, May 7, 2010 at 5:16 PM, Nitin Gupta <ngupta(a)vflare.org> wrote: > On 05/07/2010 01:33 PM, jassi brar wrote: >> On Fri, May 7, 2010 at 4:25 PM, Nitin Gupta <ngupta(a)vflare.org> wrote: >>> Added SWP_BLKDEV flag to distinguish block and regular file backed >>> swap devices. We could also check if a swap is entire block device, >>> rather than a file, by: >>> S_ISBLK(swap_info_struct->swap_file->f_mapping->host->i_mode) >>> but, I think, simply checking this flag is more convenient. >> This might make it convenient for now but is likely to increase complexity and >> redundancy. Why not define a macro/inline to figure that out? >> > > Accessing such long pointer chain is maybe not good thing to do for > every swap_entry_free() call? Simple checking a flag is perhaps slightly > faster? I also can't see how this flag can later increase complexity > compared to creating new macro for this check. I am not sure about effects on the speed, that needs to be seen. But here are points against using a new flag.... a) Defining a new flag hogs up precious real-estate of remaining just 2 bits (apparently the new flags are to go before SWP_SCANNING) b) Over time, some dependent code might evolve to depend upon this flag, while others might use the indirection to deduct if it is block device or not. That will make it complicated to make any relevant change in future as we'll have to keep track of more than one way to check the status. Creating a new macro doesn't create a new path to reach the status, but a FLAG does. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Pekka Enberg on 7 May 2010 05:30 On Fri, May 7, 2010 at 5:16 PM, Nitin Gupta <ngupta(a)vflare.org> wrote: >> On 05/07/2010 01:33 PM, jassi brar wrote: >>> On Fri, May 7, 2010 at 4:25 PM, Nitin Gupta <ngupta(a)vflare.org> wrote: >>>> Added SWP_BLKDEV flag to distinguish block and regular file backed >>>> swap devices. We could also check if a swap is entire block device, >>>> rather than a file, by: >>>> S_ISBLK(swap_info_struct->swap_file->f_mapping->host->i_mode) >>>> but, I think, simply checking this flag is more convenient. >>> This might make it convenient for now but is likely to increase complexity and >>> redundancy. Why not define a macro/inline to figure that out? >>> >> >> Accessing such long pointer chain is maybe not good thing to do for >> every swap_entry_free() call? Simple checking a flag is perhaps slightly >> faster? I also can't see how this flag can later increase complexity >> compared to creating new macro for this check. > > �I am not sure about effects on the speed, that needs to be seen. > �But here are points against using a new flag.... On Fri, May 7, 2010 at 11:56 AM, jassi brar <jassisinghbrar(a)gmail.com> wrote: > a) Defining a new flag hogs up precious real-estate of remaining just 2 bits > � (apparently the new flags are to go before SWP_SCANNING) > > b) Over time, some dependent code might evolve to depend upon this flag, while > � � others might use the indirection to deduct if it is block device > or not. That > � �will make it complicated to make any relevant change in future as we'll have > � �to keep track of more than one way to check the status. Creating a new > � macro doesn't create a new path to reach the status, but a FLAG does. I think the flag is cleaner and I'm not convinced by the above speculative arguments that we should switch to a static inline function. Hugh, Linus, would you mind voicing your opinion which way to go here? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Nigel Cunningham on 7 May 2010 05:40
Hi again. On 07/05/10 17:25, Nitin Gupta wrote: > Added SWP_BLKDEV flag to distinguish block and regular file backed > swap devices. We could also check if a swap is entire block device, > rather than a file, by: > S_ISBLK(swap_info_struct->swap_file->f_mapping->host->i_mode) > but, I think, simply checking this flag is more convenient. > > Signed-off-by: Nitin Gupta<ngupta(a)vflare.org> > --- > include/linux/swap.h | 1 + > mm/swapfile.c | 1 + > 2 files changed, 2 insertions(+), 0 deletions(-) > > diff --git a/include/linux/swap.h b/include/linux/swap.h > index 1f59d93..ec2b7a4 100644 > --- a/include/linux/swap.h > +++ b/include/linux/swap.h > @@ -146,6 +146,7 @@ enum { > SWP_DISCARDING = (1<< 3), /* now discarding a free cluster */ > SWP_SOLIDSTATE = (1<< 4), /* blkdev seeks are cheap */ > SWP_CONTINUED = (1<< 5), /* swap_map has count continuation */ > + SWP_BLKDEV = (1<< 6), /* its a block device */ > /* add others here before... */ > SWP_SCANNING = (1<< 8), /* refcount in scan_swap_map */ > }; > diff --git a/mm/swapfile.c b/mm/swapfile.c > index 6cd0a8f..ecb069e 100644 > --- a/mm/swapfile.c > +++ b/mm/swapfile.c > @@ -1884,6 +1884,7 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) > if (error< 0) > goto bad_swap; > p->bdev = bdev; > + p->flags |= SWP_BLKDEV; > } else if (S_ISREG(inode->i_mode)) { > p->bdev = inode->i_sb->s_bdev; > mutex_lock(&inode->i_mutex); The more I read your patches, the more I think either I'm seriously confused (entirely possible!) or you are. Don't you want to distinguish RAM backed swap from swap that's either a partition or a file? If that's the case, you should also be setting SWP_BLKDEV in the S_ISREG part that follows iff the p->bdev is a regular file. Regards, Nigel -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |