From: Jan Kara <jack@suse.cz>
To: Al Viro <viro@ZenIV.linux.org.uk>
Cc: Jan Kara <jack@suse.cz>, linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH] fs: Move check for mappings without pages from iterate_bdevs()
Date: Thu, 27 Sep 2012 11:31:05 +0200 [thread overview]
Message-ID: <20120927093105.GA28126@quack.suse.cz> (raw)
In-Reply-To: <20120927052837.GA22552@ZenIV.linux.org.uk>
On Thu 27-09-12 06:28:37, Al Viro wrote:
> On Tue, Sep 25, 2012 at 11:22:07AM +0200, Jan Kara wrote:
> > Currently, iterate_bdevs() skips block devices without any pages in page
> > cache. That is fine for current use by sync(2) but may be rather surprising
> > for possible future users. So move the checks from iterate_bdevs() to
> > callback functions used by sync(2).
>
> *snort*
>
> You know, testing is occasionally useful. Sure, it's boring, but once in
> a while one gets amusing results.
Heh, stupid me. Sorry for that. The patch looked *so* obviously correct
;)...
> The thing is, the only reason why the
> sucker hadn't oopsed *without* that patch was that the only non-bdev on that
> inode list happened to have zero in ->mapping->nr_pages. Reliably.
> What we'd accidentally avoided (until that patch) was stepping on the
> root directory of bdev filesystem. I_BDEV() on it is fine - it's allocated
> in a regular way, so we are not doing anything bad with container_of() here.
> However, it never went through bdget(), obviously - just new_inode(). And
> it has I_BDEV(inode)->bd_inode == NULL. The rest is obious...
>
> More interesting question is whether inode list is the right approach. After
> all, there's a list with suggestive name - all_bdevs...
And I completely missed that list. It looks like a better match for my
needs. I'll change the function to iterate over that list. Thanks for the
pointer.
Honza
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR
prev parent reply other threads:[~2012-09-27 9:31 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-09-25 9:22 [PATCH] fs: Move check for mappings without pages from iterate_bdevs() Jan Kara
2012-09-27 5:28 ` Al Viro
2012-09-27 9:31 ` Jan Kara [this message]
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=20120927093105.GA28126@quack.suse.cz \
--to=jack@suse.cz \
--cc=linux-fsdevel@vger.kernel.org \
--cc=viro@ZenIV.linux.org.uk \
/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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).