VFS: introduce helpers for the s_dirty flag
in [Kernel]
|
Prev: [PATCH 5/6] cciss: add performant mode support
Next: [PATCH 6/6] cciss: new controller support and bump driver version
From: Andrew Morton on 28 May 2010 16:30 On Tue, 25 May 2010 16:48:56 +0300 Artem Bityutskiy <dedekind1(a)gmail.com> wrote: > From: Artem Bityutskiy <Artem.Bityutskiy(a)nokia.com> > > This patch introduces 3 new VFS helpers: 'mark_sb_dirty()', > 'mark_sb_clean()', and 'is_sb_dirty()'. The helpers simply > set 'sb->s_dirt' or test 'sb->s_dirt'. The plan is to make > every FS use these helpers instead of manipulating the > 'sb->s_dirt' flag directly. > > Ultimately, this change is a preparation for the periodic > superblock synchronization optimization which is about > preventing the "sync_supers" kernel thread from waking up > even if there is nothing to synchronize. > > This patch also makes VFS use the new helpers. Patchset generally looks good to me. But I don't like the names :( > +static inline void mark_sb_dirty(struct super_block *sb) > +{ > + sb->s_dirt = 1; > +} > +static inline void mark_sb_clean(struct super_block *sb) > +{ > + sb->s_dirt = 0; > +} > +static inline int is_sb_dirty(struct super_block *sb) > +{ > + return sb->s_dirt; > +} A more conventional and superior naming scheme is subsystemid_specific_function_identifier(). eg, bio_add_page() instead of add_page_to_bio(). So these want to be sb_mark_dirty(), etc. Being very old code written by very yound people, the VFS kinda ignores that convention, but it doesn't hurt to use it for new code. Feel free to ignore me if that's too much of a PITA ;) -- 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: Al Viro on 28 May 2010 17:20 On Fri, May 28, 2010 at 01:23:18PM -0700, Andrew Morton wrote: > A more conventional and superior naming scheme is > subsystemid_specific_function_identifier(). eg, bio_add_page() instead > of add_page_to_bio(). > > So these want to be sb_mark_dirty(), etc. > > Being very old code written by very yound people, the VFS kinda ignores > that convention, but it doesn't hurt to use it for new code. > > Feel free to ignore me if that's too much of a PITA ;) The real issue is that it's almost certainly an overdesign. Let's get rid of the bogus uses first and figure out what's happening in what remains, OK? I have no problems with doing such wrappers, but if we touch every place using ->s_dirt anyway, let's at least take a good look at them. I'm mostly OK with what had emerged for the final patch in series, but... -- 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: Andrew Morton on 28 May 2010 17:30 On Fri, 28 May 2010 22:14:32 +0100 Al Viro <viro(a)ZenIV.linux.org.uk> wrote: > On Fri, May 28, 2010 at 01:23:18PM -0700, Andrew Morton wrote: > > > A more conventional and superior naming scheme is > > subsystemid_specific_function_identifier(). eg, bio_add_page() instead > > of add_page_to_bio(). > > > > So these want to be sb_mark_dirty(), etc. > > > > Being very old code written by very yound people, the VFS kinda ignores > > that convention, but it doesn't hurt to use it for new code. > > > > Feel free to ignore me if that's too much of a PITA ;) > > The real issue is that it's almost certainly an overdesign. Let's > get rid of the bogus uses first and figure out what's happening in > what remains, OK? That would be good. > I have no problems with doing such wrappers, but if we touch every > place using ->s_dirt anyway, let's at least take a good look at them. When adding wrappers we should also rename ->s_dirt (say, to __s_dirt) to catch out any unconverted code. -- 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: Artem Bityutskiy on 29 May 2010 04:00 On Fri, 2010-05-28 at 13:23 -0700, Andrew Morton wrote: > > +static inline void mark_sb_dirty(struct super_block *sb) > > +{ > > + sb->s_dirt = 1; > > +} > > +static inline void mark_sb_clean(struct super_block *sb) > > +{ > > + sb->s_dirt = 0; > > +} > > +static inline int is_sb_dirty(struct super_block *sb) > > +{ > > + return sb->s_dirt; > > +} > > A more conventional and superior naming scheme is > subsystemid_specific_function_identifier(). eg, bio_add_page() instead > of add_page_to_bio(). > > So these want to be sb_mark_dirty(), etc. > > Being very old code written by very yound people, the VFS kinda ignores > that convention, but it doesn't hurt to use it for new code. > > Feel free to ignore me if that's too much of a PITA ;) Sure I'll re-name them, thanks! -- Best Regards, Artem Bityutskiy (Артём Битюцкий) -- 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: Artem Bityutskiy on 29 May 2010 04:20
On Fri, 2010-05-28 at 14:17 -0700, Andrew Morton wrote: > On Fri, 28 May 2010 22:14:32 +0100 > Al Viro <viro(a)ZenIV.linux.org.uk> wrote: > > > On Fri, May 28, 2010 at 01:23:18PM -0700, Andrew Morton wrote: > > > > > A more conventional and superior naming scheme is > > > subsystemid_specific_function_identifier(). eg, bio_add_page() instead > > > of add_page_to_bio(). > > > > > > So these want to be sb_mark_dirty(), etc. > > > > > > Being very old code written by very yound people, the VFS kinda ignores > > > that convention, but it doesn't hurt to use it for new code. > > > > > > Feel free to ignore me if that's too much of a PITA ;) > > > > The real issue is that it's almost certainly an overdesign. Let's > > get rid of the bogus uses first and figure out what's happening in > > what remains, OK? > > That would be good. Yes, I just mechanically introduced the wrappers to all FS-es. But as per Al's request, I am going to try looking at how FSwe use it and validate the usage. It'll take some time as this stuff is my background task. Will see. > > I have no problems with doing such wrappers, but if we touch every > > place using ->s_dirt anyway, let's at least take a good look at them. > > When adding wrappers we should also rename ->s_dirt (say, to __s_dirt) > to catch out any unconverted code. Right, I did this in the following patch: [PATCHv4 16/17] VFS: rename s_dirt to s_dirty I thought that adding a leading '_' is not very neat, so added 'y' at the end. -- Best Regards, Artem Bityutskiy (Артём Битюцкий) -- 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/ |