Re: [PATCH v2 15/30] xfs: Define usercopy region in xfs_inode slab cache

2017-08-30 Thread Christoph Hellwig
On Wed, Aug 30, 2017 at 06:05:58PM +1000, Dave Chinner wrote: > Ok, that's sounds like it'll fit right in with what I've been > prototyping for the extent code in xfs_bmap.c. I can make that work > with a cursor-based lookup/inc/dec/ins/del API similar to the bmbt > API. I've been looking to abstra

Re: [PATCH v2 15/30] xfs: Define usercopy region in xfs_inode slab cache

2017-08-30 Thread Dave Chinner
On Wed, Aug 30, 2017 at 12:14:03AM -0700, Christoph Hellwig wrote: > On Wed, Aug 30, 2017 at 07:51:57AM +1000, Dave Chinner wrote: > > Right, I've looked at btrees, too, but it's more complex than just > > using an rbtree. I originally looked at using Peter Z's old > > RCU-aware btree code, but it

Re: [PATCH v2 15/30] xfs: Define usercopy region in xfs_inode slab cache

2017-08-30 Thread Christoph Hellwig
On Wed, Aug 30, 2017 at 07:51:57AM +1000, Dave Chinner wrote: > Right, I've looked at btrees, too, but it's more complex than just > using an rbtree. I originally looked at using Peter Z's old > RCU-aware btree code, but it doesn't hold data in the tree leaves. > So that needed significant modifica

Re: [PATCH v2 15/30] xfs: Define usercopy region in xfs_inode slab cache

2017-08-29 Thread Kees Cook
On Tue, Aug 29, 2017 at 3:15 PM, Dave Chinner wrote: > If you are touching multiple filesystems, you really should cc the > entire patchset to linux-fsdevel, similar to how you sent the entire > patchset to lkml. That way the entire series will end up on a list > that almost all fs developers read

Re: [PATCH v2 15/30] xfs: Define usercopy region in xfs_inode slab cache

2017-08-29 Thread Dave Chinner
On Tue, Aug 29, 2017 at 11:48:49AM -0700, Kees Cook wrote: > On Mon, Aug 28, 2017 at 9:47 PM, Darrick J. Wong > wrote: > > On Mon, Aug 28, 2017 at 02:57:14PM -0700, Kees Cook wrote: > >> On Mon, Aug 28, 2017 at 2:49 PM, Darrick J. Wong > >> wrote: > >> > On Mon, Aug 28, 2017 at 02:34:56PM -0700,

Re: [PATCH v2 15/30] xfs: Define usercopy region in xfs_inode slab cache

2017-08-29 Thread Dave Chinner
On Tue, Aug 29, 2017 at 05:45:36AM -0700, Christoph Hellwig wrote: > On Tue, Aug 29, 2017 at 10:31:26PM +1000, Dave Chinner wrote: > > Probably should. I've already been looking at killing the inline > > extents array to simplify the management of the extent list (much > > simpler to index by rbtr

Re: [PATCH v2 15/30] xfs: Define usercopy region in xfs_inode slab cache

2017-08-29 Thread Darrick J. Wong
On Tue, Aug 29, 2017 at 11:48:49AM -0700, Kees Cook wrote: > On Mon, Aug 28, 2017 at 9:47 PM, Darrick J. Wong > wrote: > > On Mon, Aug 28, 2017 at 02:57:14PM -0700, Kees Cook wrote: > >> On Mon, Aug 28, 2017 at 2:49 PM, Darrick J. Wong > >> wrote: > >> > On Mon, Aug 28, 2017 at 02:34:56PM -0700,

Re: [PATCH v2 15/30] xfs: Define usercopy region in xfs_inode slab cache

2017-08-29 Thread Kees Cook
On Tue, Aug 29, 2017 at 1:14 AM, Christoph Hellwig wrote: > One thing I've been wondering is wether we should actually just > get rid of the online area. Compared to reading an inode from > disk a single additional kmalloc is negligible, and not having the > inline data / extent list would allow

Re: [PATCH v2 15/30] xfs: Define usercopy region in xfs_inode slab cache

2017-08-29 Thread Kees Cook
On Mon, Aug 28, 2017 at 9:47 PM, Darrick J. Wong wrote: > On Mon, Aug 28, 2017 at 02:57:14PM -0700, Kees Cook wrote: >> On Mon, Aug 28, 2017 at 2:49 PM, Darrick J. Wong >> wrote: >> > On Mon, Aug 28, 2017 at 02:34:56PM -0700, Kees Cook wrote: >> >> From: David Windsor >> >> >> >> The XFS inline

Re: [PATCH v2 15/30] xfs: Define usercopy region in xfs_inode slab cache

2017-08-29 Thread Christoph Hellwig
On Tue, Aug 29, 2017 at 10:31:26PM +1000, Dave Chinner wrote: > Probably should. I've already been looking at killing the inline > extents array to simplify the management of the extent list (much > simpler to index by rbtree when we don't have direct/indirect > structures), so killing the inline

Re: [PATCH v2 15/30] xfs: Define usercopy region in xfs_inode slab cache

2017-08-29 Thread Dave Chinner
On Tue, Aug 29, 2017 at 01:14:53AM -0700, Christoph Hellwig wrote: > One thing I've been wondering is wether we should actually just > get rid of the online area. Compared to reading an inode from > disk a single additional kmalloc is negligible, and not having the > inline data / extent list woul

Re: [PATCH v2 15/30] xfs: Define usercopy region in xfs_inode slab cache

2017-08-29 Thread Christoph Hellwig
One thing I've been wondering is wether we should actually just get rid of the online area. Compared to reading an inode from disk a single additional kmalloc is negligible, and not having the inline data / extent list would allow us to reduce the inode size significantly. Kees/David: how many o

Re: [PATCH v2 15/30] xfs: Define usercopy region in xfs_inode slab cache

2017-08-28 Thread Darrick J. Wong
On Mon, Aug 28, 2017 at 02:57:14PM -0700, Kees Cook wrote: > On Mon, Aug 28, 2017 at 2:49 PM, Darrick J. Wong > wrote: > > On Mon, Aug 28, 2017 at 02:34:56PM -0700, Kees Cook wrote: > >> From: David Windsor > >> > >> The XFS inline inode data, stored in struct xfs_inode_t field > >> i_df.if_u2.if

Re: [PATCH v2 15/30] xfs: Define usercopy region in xfs_inode slab cache

2017-08-28 Thread Kees Cook
On Mon, Aug 28, 2017 at 2:49 PM, Darrick J. Wong wrote: > On Mon, Aug 28, 2017 at 02:34:56PM -0700, Kees Cook wrote: >> From: David Windsor >> >> The XFS inline inode data, stored in struct xfs_inode_t field >> i_df.if_u2.if_inline_data and therefore contained in the xfs_inode slab >> cache, need

Re: [PATCH v2 15/30] xfs: Define usercopy region in xfs_inode slab cache

2017-08-28 Thread Darrick J. Wong
On Mon, Aug 28, 2017 at 02:34:56PM -0700, Kees Cook wrote: > From: David Windsor > > The XFS inline inode data, stored in struct xfs_inode_t field > i_df.if_u2.if_inline_data and therefore contained in the xfs_inode slab > cache, needs to be copied to/from userspace. > > cache object allocation:

[PATCH v2 15/30] xfs: Define usercopy region in xfs_inode slab cache

2017-08-28 Thread Kees Cook
From: David Windsor The XFS inline inode data, stored in struct xfs_inode_t field i_df.if_u2.if_inline_data and therefore contained in the xfs_inode slab cache, needs to be copied to/from userspace. cache object allocation: fs/xfs/xfs_icache.c: xfs_inode_alloc(...): ...