Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1161581AbbKSUyh (ORCPT ); Thu, 19 Nov 2015 15:54:37 -0500 Received: from mail-wm0-f49.google.com ([74.125.82.49]:38361 "EHLO mail-wm0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758947AbbKSUyf (ORCPT ); Thu, 19 Nov 2015 15:54:35 -0500 MIME-Version: 1.0 In-Reply-To: <20151119155525.GB13055@bfoster.bfoster> References: <1447800381-20167-1-git-send-email-octavian.purdila@intel.com> <20151119155525.GB13055@bfoster.bfoster> Date: Thu, 19 Nov 2015 22:54:34 +0200 Message-ID: Subject: Re: [RFC PATCH] xfs: support for non-mmu architectures From: Octavian Purdila To: Brian Foster Cc: xfs@oss.sgi.com, linux-fsdevel , lkml Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3321 Lines: 81 On Thu, Nov 19, 2015 at 5:55 PM, Brian Foster wrote: > On Wed, Nov 18, 2015 at 12:46:21AM +0200, Octavian Purdila wrote: >> Naive implementation for non-mmu architectures: allocate physically >> contiguous xfs buffers with alloc_pages. Terribly inefficient with >> memory and fragmentation on high I/O loads but it may be good enough >> for basic usage (which most non-mmu architectures will need). >> >> This patch was tested with lklfuse [1] and basic operations seems to >> work even with 16MB allocated for LKL. >> >> [1] https://github.com/lkl/linux >> >> Signed-off-by: Octavian Purdila >> --- > > Interesting, though this makes me wonder why we couldn't have a new > _XBF_VMEM (for example) buffer type that uses vmalloc(). I'm not > familiar with mmu-less context, but I see that mm/nommu.c has a > __vmalloc() interface that looks like it ultimately translates into an > alloc_pages() call. Would that accomplish what this patch is currently > trying to do? > Hi Brian, Ah, that sounds nice! We could get rid of the #ifdefs and use a common path in that case. vmalloc should work on non-mmu and it seems that there is a vmalloc_to_page() we can use to get the physical pages. I'll give it a try. Is there a central place where we could enable the new _XBF_VMEM to be the default for non-mmu arches? > I ask because it seems like that would help clean up the code a bit, for > one. It might also facilitate some degree of testing of the XFS bits > (even if utilized sparingly in DEBUG mode if it weren't suitable enough > for generic/mmu use). We currently allocate and map the buffer pages > separately and I'm not sure if there's any particular reasons for doing > that outside of some congestion handling in the allocation code and > XBF_UNMAPPED buffers, the latter probably being irrelevant for nommu. > Any other thoughts on that? > >> fs/xfs/xfs_buf.c | 27 +++++++++++++++++++++++++++ >> 1 file changed, 27 insertions(+) >> >> diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c >> index 8ecffb3..50b5246 100644 >> --- a/fs/xfs/xfs_buf.c >> +++ b/fs/xfs/xfs_buf.c > ... >> @@ -816,11 +835,19 @@ xfs_buf_get_uncached( >> if (error) >> goto fail_free_buf; >> >> +#ifdef CONFIG_MMU >> for (i = 0; i < page_count; i++) { >> bp->b_pages[i] = alloc_page(xb_to_gfp(flags)); >> if (!bp->b_pages[i]) >> goto fail_free_mem; >> } >> +#else >> + bp->b_pages[0] = alloc_pages(flags, order_base_2(page_count)); >> + if (!bp->b_pages[0]) >> + goto fail_free_buf; >> + for (i = 1; i < page_count; i++) >> + bp->b_pages[i] = bp->b_pages[i-1] + 1; >> +#endif > > We still have a path into __free_page() further down in this function if > _xfs_buf_map_pages() fails. Granted, _xfs_buf_map_pages() should > probably never fail in this case, but it still looks like a land mine at > the very least. > OK. Adding a i = 1; right before the #endif should take care of that, if the vmalloc approach does not work. -- 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/