From mboxrd@z Thu Jan 1 00:00:00 1970 From: Al Viro Subject: Re: [PATCH] fs: Move check for mappings without pages from iterate_bdevs() Date: Thu, 27 Sep 2012 06:28:37 +0100 Message-ID: <20120927052837.GA22552@ZenIV.linux.org.uk> References: <1348564927-11918-1-git-send-email-jack@suse.cz> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: linux-fsdevel@vger.kernel.org To: Jan Kara Return-path: Received: from zeniv.linux.org.uk ([195.92.253.2]:51438 "EHLO ZenIV.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754015Ab2I0F2j (ORCPT ); Thu, 27 Sep 2012 01:28:39 -0400 Content-Disposition: inline In-Reply-To: <1348564927-11918-1-git-send-email-jack@suse.cz> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: 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. 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...