From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kara Subject: Re: [PATCH] fs: Move check for mappings without pages from iterate_bdevs() Date: Thu, 27 Sep 2012 11:31:05 +0200 Message-ID: <20120927093105.GA28126@quack.suse.cz> References: <1348564927-11918-1-git-send-email-jack@suse.cz> <20120927052837.GA22552@ZenIV.linux.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Jan Kara , linux-fsdevel@vger.kernel.org To: Al Viro Return-path: Received: from cantor2.suse.de ([195.135.220.15]:59688 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751561Ab2I0JbI (ORCPT ); Thu, 27 Sep 2012 05:31:08 -0400 Content-Disposition: inline In-Reply-To: <20120927052837.GA22552@ZenIV.linux.org.uk> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: 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 SUSE Labs, CR