Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752623AbaKVUED (ORCPT ); Sat, 22 Nov 2014 15:04:03 -0500 Received: from mail-pd0-f173.google.com ([209.85.192.173]:36849 "EHLO mail-pd0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751409AbaKVUEA (ORCPT ); Sat, 22 Nov 2014 15:04:00 -0500 Date: Sat, 22 Nov 2014 12:03:57 -0800 From: Omar Sandoval To: David Sterba Cc: Alexander Viro , Andrew Morton , Chris Mason , Josef Bacik , linux-btrfs@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, linux-nfs@vger.kernel.org, Trond Myklebust , Mel Gorman Subject: Re: [PATCH v2 5/5] btrfs: enable swap file support Message-ID: <20141122200357.GA15189@mew> References: <20141121180045.GF8568@twin.jikos.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20141121180045.GF8568@twin.jikos.cz> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Nov 21, 2014 at 07:00:45PM +0100, David Sterba wrote: > > + pr_err("BTRFS: swapfile has holes"); > > + ret = -EINVAL; > > + goto out; > > + } > > + if (em->block_start == EXTENT_MAP_INLINE) { > > + pr_err("BTRFS: swapfile is inline"); > > While the test is valid, this would mean that the file is smaller than > the inline limit, which is now one page. I think the generic swap code > would refuse such a small file anyway. > Sure. This test doesn't really cost us anything, so I think I'd feel a little better just leaving it in. I'll add a comment for the next close reader. Besides that and Filipe's response, I'll address everything you mentioned here and in your other email in the next version, thanks. > > + ret = -EINVAL; > > + goto out; > > + } > > + if (test_bit(EXTENT_FLAG_COMPRESSED, &em->flags)) { > > + pr_err("BTRFS: swapfile is compresed"); > > + ret = -EINVAL; > > + goto out; > > + } > > I think the preallocated extents should be refused as well. This means > the filesystem has enough space to hold the data but it would still have > to go through the allocation and could in turn stress the memory > management code that triggered the swapping activity in the first place. > > Though it's probably still possible to reach such corner case even with > fully allocated nodatacow file, this should be reviewed anyway. > I'll definitely take a closer look at this. In particular, btrfs_get_blocks_direct and btrfs_get_extent do allocations in some cases which I'll look into. -- Omar -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/