From: Dave Chinner <david@fromorbit.com> To: "Rafael J. Wysocki" <rafael@kernel.org> Cc: "Luis R. Rodriguez" <mcgrof@kernel.org>, Al Viro <viro@zeniv.linux.org.uk>, bart.vanassche@wdc.com, ming.lei@redhat.com, Ted Ts'o <tytso@mit.edu>, "Darrick J. Wong" <darrick.wong@oracle.com>, Jiri Kosina <jikos@kernel.org>, "Rafael J. Wysocki" <rjw@rjwysocki.net>, Pavel Machek <pavel@ucw.cz>, Len Brown <len.brown@intel.com>, linux-fsdevel@vger.kernel.org, Boris Ostrovsky <boris.ostrovsky@oracle.com>, Juergen Gross <jgross@suse.com>, Todd Brandt <todd.e.brandt@linux.intel.com>, nborisov@suse.com, Jan Kara <jack@suse.cz>, "Martin K. Petersen" <martin.petersen@oracle.com>, Oliver Neukum <ONeukum@suse.com>, oleksandr@natalenko.name, Oleg Antonyan <oleg.b.antonyan@gmail.com>, Yu Chen <yu.chen.surf@gmail.com>, Dan Williams <dan.j.williams@intel.com>, Linux PM <linux-pm@vger.kernel.org>, linux-block@vger.kernel.org, linux-xfs@vger.kernel.org, Linux Kernel Mailing List <linux-kernel@vger.kernel.org> Subject: Re: [PATCH 05/11] fs: add iterate_supers_excl() and iterate_supers_reverse_excl() Date: Thu, 30 Nov 2017 12:34:06 +1100 [thread overview] Message-ID: <20171130013406.GM5858@dastard> (raw) In-Reply-To: <CAJZ5v0gO49=pXGEdi42Hka=Rf=SqVWp-QSJf9_+bGuOgsHMFig@mail.gmail.com> On Thu, Nov 30, 2017 at 12:48:15AM +0100, Rafael J. Wysocki wrote: > On Thu, Nov 30, 2017 at 12:23 AM, Luis R. Rodriguez <mcgrof@kernel.org> wrote: > > There are use cases where we wish to traverse the superblock list > > but also capture errors, and in which case we want to avoid having > > our callers issue a lock themselves since we can do the locking for > > the callers. Provide a iterate_supers_excl() which calls a function > > with the write lock held. If an error occurs we capture it and > > propagate it. > > > > Likewise there are use cases where we wish to traverse the superblock > > list but in reverse order. The new iterate_supers_reverse_excl() helpers > > does this but also also captures any errors encountered. > > > > Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org> > > --- > > fs/super.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > include/linux/fs.h | 2 ++ > > 2 files changed, 93 insertions(+) > > > > diff --git a/fs/super.c b/fs/super.c > > index a63513d187e8..885711c1d35b 100644 > > --- a/fs/super.c > > +++ b/fs/super.c > > @@ -605,6 +605,97 @@ void iterate_supers(void (*f)(struct super_block *, void *), void *arg) > > spin_unlock(&sb_lock); > > } > > > > +/** > > + * iterate_supers_excl - exclusively call func for all active superblocks > > + * @f: function to call > > + * @arg: argument to pass to it > > + * > > + * Scans the superblock list and calls given function, passing it > > + * locked superblock and given argument. Returns 0 unless an error > > + * occurred on calling the function on any superblock. > > + */ > > +int iterate_supers_excl(int (*f)(struct super_block *, void *), void *arg) > > +{ > > + struct super_block *sb, *p = NULL; > > + int error = 0; > > + > > + spin_lock(&sb_lock); > > + list_for_each_entry(sb, &super_blocks, s_list) { > > + if (hlist_unhashed(&sb->s_instances)) > > + continue; > > + sb->s_count++; > > + spin_unlock(&sb_lock); > > Can anything bad happen if the list is modified at this point by a > concurrent thread? No. We have a valid reference to sb->s_count and that keeps it on the list while we have the lock dropped. The sb reference isn't dropped until we've iterated to the next sb on the list and taken a reference to that, hence it's safe to drop and regain the list lock without needing to restart the iteration. > > + > > + down_write(&sb->s_umount); > > + if (sb->s_root && (sb->s_flags & SB_BORN)) { > > + error = f(sb, arg); > > + if (error) { > > + up_write(&sb->s_umount); > > + spin_lock(&sb_lock); > > + __put_super(sb); > > + break; > > + } > > + } > > + up_write(&sb->s_umount); > > + > > + spin_lock(&sb_lock); > > + if (p) > > + __put_super(p); > > + p = sb; This code here is what drops the reference to the previous sb we've iterated past. FWIW, this "hold until next is held" iteration pattern is used frequently for inodes, dentries, and other reference counted VFS objects so we can iterate the list without needing to hold the list lock for the entire iteration.... Cheers, Dave. -- Dave Chinner david@fromorbit.com
WARNING: multiple messages have this Message-ID (diff)
From: Dave Chinner <david@fromorbit.com> To: "Rafael J. Wysocki" <rafael@kernel.org> Cc: "Luis R. Rodriguez" <mcgrof@kernel.org>, Al Viro <viro@zeniv.linux.org.uk>, bart.vanassche@wdc.com, ming.lei@redhat.com, Ted Ts'o <tytso@mit.edu>, "Darrick J. Wong" <darrick.wong@oracle.com>, Jiri Kosina <jikos@kernel.org>, "Rafael J. Wysocki" <rjw@rjwysocki.net>, Pavel Machek <pavel@ucw.cz>, Len Brown <len.brown@intel.com>, linux-fsdevel@vger.kernel.org, Boris Ostrovsky <boris.ostrovsky@oracle.com>, Juergen Gross <jgross@suse.com>, Todd Brandt <todd.e.brandt@linux.intel.com>, nborisov@suse.com, Jan Kara <jack@suse.cz>, "Martin K. Petersen" <martin.petersen@oracle.com>, Oliver Neukum <ONeukum@suse.com>, oleksandr@natalenko.name, Oleg Antonyan <oleg.b.antonyan@gmail.com>, Yu Chen <yu.chen.surf@gmail.com>, Dan Williams <dan.j.williams@intel.com>, Linux Subject: Re: [PATCH 05/11] fs: add iterate_supers_excl() and iterate_supers_reverse_excl() Date: Thu, 30 Nov 2017 12:34:06 +1100 [thread overview] Message-ID: <20171130013406.GM5858@dastard> (raw) In-Reply-To: <CAJZ5v0gO49=pXGEdi42Hka=Rf=SqVWp-QSJf9_+bGuOgsHMFig@mail.gmail.com> On Thu, Nov 30, 2017 at 12:48:15AM +0100, Rafael J. Wysocki wrote: > On Thu, Nov 30, 2017 at 12:23 AM, Luis R. Rodriguez <mcgrof@kernel.org> wrote: > > There are use cases where we wish to traverse the superblock list > > but also capture errors, and in which case we want to avoid having > > our callers issue a lock themselves since we can do the locking for > > the callers. Provide a iterate_supers_excl() which calls a function > > with the write lock held. If an error occurs we capture it and > > propagate it. > > > > Likewise there are use cases where we wish to traverse the superblock > > list but in reverse order. The new iterate_supers_reverse_excl() helpers > > does this but also also captures any errors encountered. > > > > Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org> > > --- > > fs/super.c | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > include/linux/fs.h | 2 ++ > > 2 files changed, 93 insertions(+) > > > > diff --git a/fs/super.c b/fs/super.c > > index a63513d187e8..885711c1d35b 100644 > > --- a/fs/super.c > > +++ b/fs/super.c > > @@ -605,6 +605,97 @@ void iterate_supers(void (*f)(struct super_block *, void *), void *arg) > > spin_unlock(&sb_lock); > > } > > > > +/** > > + * iterate_supers_excl - exclusively call func for all active superblocks > > + * @f: function to call > > + * @arg: argument to pass to it > > + * > > + * Scans the superblock list and calls given function, passing it > > + * locked superblock and given argument. Returns 0 unless an error > > + * occurred on calling the function on any superblock. > > + */ > > +int iterate_supers_excl(int (*f)(struct super_block *, void *), void *arg) > > +{ > > + struct super_block *sb, *p = NULL; > > + int error = 0; > > + > > + spin_lock(&sb_lock); > > + list_for_each_entry(sb, &super_blocks, s_list) { > > + if (hlist_unhashed(&sb->s_instances)) > > + continue; > > + sb->s_count++; > > + spin_unlock(&sb_lock); > > Can anything bad happen if the list is modified at this point by a > concurrent thread? No. We have a valid reference to sb->s_count and that keeps it on the list while we have the lock dropped. The sb reference isn't dropped until we've iterated to the next sb on the list and taken a reference to that, hence it's safe to drop and regain the list lock without needing to restart the iteration. > > + > > + down_write(&sb->s_umount); > > + if (sb->s_root && (sb->s_flags & SB_BORN)) { > > + error = f(sb, arg); > > + if (error) { > > + up_write(&sb->s_umount); > > + spin_lock(&sb_lock); > > + __put_super(sb); > > + break; > > + } > > + } > > + up_write(&sb->s_umount); > > + > > + spin_lock(&sb_lock); > > + if (p) > > + __put_super(p); > > + p = sb; This code here is what drops the reference to the previous sb we've iterated past. FWIW, this "hold until next is held" iteration pattern is used frequently for inodes, dentries, and other reference counted VFS objects so we can iterate the list without needing to hold the list lock for the entire iteration.... Cheers, Dave. -- Dave Chinner david@fromorbit.com
next prev parent reply other threads:[~2017-11-30 1:35 UTC|newest] Thread overview: 59+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-11-29 23:23 [PATCH 00/11] fs: use freeze_fs on suspend/hibernate Luis R. Rodriguez 2017-11-29 23:23 ` [PATCH 01/11] fs: provide unlocked helper for freeze_super() Luis R. Rodriguez 2017-11-30 16:58 ` Jan Kara 2017-11-29 23:23 ` [PATCH 02/11] fs: provide unlocked helper thaw_super() Luis R. Rodriguez 2017-11-30 16:59 ` Jan Kara 2017-11-29 23:23 ` [PATCH 03/11] fs: add frozen sb state helpers Luis R. Rodriguez 2017-11-30 17:13 ` Jan Kara 2017-11-30 19:05 ` Luis R. Rodriguez 2017-12-01 11:47 ` Jan Kara 2017-12-01 21:13 ` Luis R. Rodriguez 2017-12-21 11:03 ` Jan Kara 2018-04-18 0:59 ` Luis R. Rodriguez 2018-04-18 10:12 ` Jan Kara 2018-04-20 18:49 ` Luis R. Rodriguez 2018-04-21 23:53 ` Jan Kara 2018-04-22 1:22 ` Luis R. Rodriguez 2018-04-22 2:53 ` Luis R. Rodriguez 2017-11-29 23:23 ` [PATCH 04/11] fs: distinguish between user initiated freeze and kernel initiated freeze Luis R. Rodriguez 2017-11-29 23:23 ` [PATCH 05/11] fs: add iterate_supers_excl() and iterate_supers_reverse_excl() Luis R. Rodriguez 2017-11-29 23:48 ` Rafael J. Wysocki 2017-11-29 23:48 ` Rafael J. Wysocki 2017-11-30 0:22 ` Luis R. Rodriguez 2017-11-30 0:22 ` Luis R. Rodriguez 2017-11-30 1:34 ` Dave Chinner [this message] 2017-11-30 1:34 ` Dave Chinner 2017-11-30 1:40 ` Rafael J. Wysocki 2017-11-30 1:40 ` Rafael J. Wysocki 2017-11-30 16:57 ` Jan Kara 2017-11-29 23:23 ` [PATCH 06/11] fs: freeze on suspend and thaw on resume Luis R. Rodriguez 2017-11-29 23:23 ` [PATCH 07/11] xfs: remove not needed freezing calls Luis R. Rodriguez 2017-11-30 16:21 ` Jan Kara 2017-11-30 20:32 ` Rafael J. Wysocki 2017-11-30 20:32 ` Rafael J. Wysocki 2017-11-30 23:30 ` Dave Chinner 2017-11-30 23:30 ` Dave Chinner 2017-11-30 23:40 ` Rafael J. Wysocki 2017-11-30 23:40 ` Rafael J. Wysocki 2017-11-29 23:23 ` [PATCH 08/11] ext4: " Luis R. Rodriguez 2017-11-29 23:23 ` [PATCH 09/11] f2fs: " Luis R. Rodriguez 2017-11-29 23:23 ` [PATCH 10/11] nilfs2: " Luis R. Rodriguez 2017-11-29 23:23 ` [PATCH 11/11] jfs: " Luis R. Rodriguez 2017-11-30 16:36 ` [PATCH 00/11] fs: use freeze_fs on suspend/hibernate Yu Chen 2017-11-30 16:41 ` Jiri Kosina 2017-11-30 16:50 ` Yu Chen 2017-12-01 19:05 ` Jeff Layton 2017-12-01 21:51 ` Dave Chinner 2017-11-30 17:01 ` Bart Van Assche 2017-11-30 17:01 ` Bart Van Assche 2017-11-30 19:42 ` Luis R. Rodriguez 2017-11-30 19:42 ` Luis R. Rodriguez 2017-11-30 20:53 ` Bart Van Assche 2017-11-30 20:53 ` Bart Van Assche 2017-11-30 20:53 ` Bart Van Assche 2017-11-30 21:03 ` Dave Chinner 2017-11-30 21:03 ` Dave Chinner 2017-11-30 21:51 ` Pavel Machek 2017-12-01 0:44 ` Luis R. Rodriguez 2017-12-13 1:09 ` Rafael J. Wysocki 2017-12-19 16:50 ` Luis R. Rodriguez
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20171130013406.GM5858@dastard \ --to=david@fromorbit.com \ --cc=ONeukum@suse.com \ --cc=bart.vanassche@wdc.com \ --cc=boris.ostrovsky@oracle.com \ --cc=dan.j.williams@intel.com \ --cc=darrick.wong@oracle.com \ --cc=jack@suse.cz \ --cc=jgross@suse.com \ --cc=jikos@kernel.org \ --cc=len.brown@intel.com \ --cc=linux-block@vger.kernel.org \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-pm@vger.kernel.org \ --cc=linux-xfs@vger.kernel.org \ --cc=martin.petersen@oracle.com \ --cc=mcgrof@kernel.org \ --cc=ming.lei@redhat.com \ --cc=nborisov@suse.com \ --cc=oleg.b.antonyan@gmail.com \ --cc=oleksandr@natalenko.name \ --cc=pavel@ucw.cz \ --cc=rafael@kernel.org \ --cc=rjw@rjwysocki.net \ --cc=todd.e.brandt@linux.intel.com \ --cc=tytso@mit.edu \ --cc=viro@zeniv.linux.org.uk \ --cc=yu.chen.surf@gmail.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.