linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Martin Steigerwald <martin@lichtvoll.de>
To: Matthew Wilcox <willy@infradead.org>
Cc: linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org,
	Matthew Wilcox <mawilcox@microsoft.com>
Subject: Re: XArray documentation
Date: Fri, 24 Nov 2017 19:01:31 +0100	[thread overview]
Message-ID: <2627399.jpLCoM7KBo@merkaba> (raw)
In-Reply-To: <20171124170307.GA681@bombadil.infradead.org>

Hi Matthew.

Matthew Wilcox - 24.11.17, 18:03:
> On Fri, Nov 24, 2017 at 05:50:41PM +0100, Martin Steigerwald wrote:
> > Matthew Wilcox - 24.11.17, 02:16:
> > > ======
> > > XArray
> > > ======
> > > 
> > > Overview
> > > ========
> > > 
> > > The XArray is an array of ULONG_MAX entries.  Each entry can be either
> > > a pointer, or an encoded value between 0 and LONG_MAX.  It is efficient
> > > when the indices used are densely clustered; hashing the object and
> > > using the hash as the index will not perform well.  A
> > > freshly-initialised
> > > XArray contains a NULL pointer at every index.  There is no difference
> > > between an entry which has never been stored to and an entry which has
> > > most
> > > recently had NULL stored to it.
> > 
> > I am no kernel developer (just provided a tiny bit of documentation a long
> > time ago)… but on reading into this, I missed:
> > 
> > What is it about? And what is it used for?
> > 
> > "Overview" appears to be already a description of the actual
> > implementation
> > specifics, instead of… well an overview.
> > 
> > Of course, I am sure you all know what it is for… but someone who wants to
> > learn about the kernel is likely to be confused by such a start.
[…]
> Thank you for your comment.  I'm clearly too close to it because even
> after reading your useful critique, I'm not sure what to change.  Please
> help me!

And likely I am too far away to and do not understand enough of it to provide 
more concrete suggestions, but let me try. (I do understand some programming 
stuff like what an array is, what a pointer what an linked list or a tree is 
or… so I am not completely novice here. I think the documentation should not 
cover any of these basics.)

> Maybe it's that I've described the abstraction as if it's the
> implementation and put too much detail into the overview.  This might
> be clearer?
> 
> The XArray is an abstract data type which behaves like an infinitely
> large array of pointers.  The index into the array is an unsigned long.
> A freshly-initialised XArray contains a NULL pointer at every index.

Yes, I think this is clearer already.

Maybe with a few sentences on "Why does the kernel provide this?", "Where is 
it used?" (if already known), "What use case is it suitable for – if I want to 
implement something into the kernel (or in user space?) ?" and probably "How 
does it differ from user data structures the kernel provides?"

I don´t know whether the questions make sense to you. But that were questions 
I had in mind as I read into your documentation. I do not think this needs to 
be long or so… maybe just a few sentences that put XArray into a context, 
before diving into the details. I think that could help new developers who 
want to learn about kernel development when they learn about XArray.

And then as you suggest all the important implementation details.

> ----
> and then move all this information into later paragraphs:
> 
> There is no difference between an entry which has never been stored to
> and an entry which has most recently had NULL stored to it.
> Each entry in the array can be either a pointer, or an
> encoded value between 0 and LONG_MAX.
> While you can use any index, the implementation is efficient when the
> indices used are densely clustered; hashing the object and using the
> hash as the index will not perform well.

Yes.

And I notice now that you have some use case remarks in here… like "efficient 
when densely clustered". I missed these initially.

Thanks,
-- 
Martin

  reply	other threads:[~2017-11-24 18:01 UTC|newest]

Thread overview: 79+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-22 21:06 [PATCH 00/62] XArray November 2017 Edition Matthew Wilcox
2017-11-22 21:06 ` [PATCH 01/62] tools: Make __test_and_clear_bit available Matthew Wilcox
2017-11-22 21:06 ` [PATCH 02/62] radix tree test suite: Remove ARRAY_SIZE Matthew Wilcox
2017-11-22 21:06 ` [PATCH 03/62] radix tree test suite: Check reclaim bit Matthew Wilcox
2017-11-22 21:06 ` [PATCH 04/62] idr test suite: Fix ida_test_random() Matthew Wilcox
2017-11-22 21:06 ` [PATCH 05/62] radix tree: Add a missing cast to gfp_t Matthew Wilcox
2017-11-22 21:28   ` Luc Van Oostenryck
2017-11-22 22:24     ` Matthew Wilcox
2017-11-22 22:35       ` Luc Van Oostenryck
2017-11-22 21:06 ` [PATCH 06/62] idr: Make cursor explicit for cyclic allocation Matthew Wilcox
2017-11-22 21:06 ` [PATCH 07/62] idr: Rewrite extended IDR API Matthew Wilcox
2017-11-22 21:06 ` [PATCH 08/62] Explicitly include radix-tree.h Matthew Wilcox
2017-11-22 21:06 ` [PATCH 09/62] arm64: Turn flush_dcache_mmap_lock into a no-op Matthew Wilcox
2017-11-22 21:06 ` [PATCH 10/62] unicore32: " Matthew Wilcox
2017-11-22 21:06 ` [PATCH 11/62] Export __set_page_dirty Matthew Wilcox
2017-11-22 21:06 ` [PATCH 12/62] xfs: Rename xa_ elements to ail_ Matthew Wilcox
2017-11-22 21:06 ` [PATCH 13/62] fscache: Use appropriate radix tree accessors Matthew Wilcox
2017-11-22 21:06 ` [PATCH 14/62] xarray: Add the xa_lock to the radix_tree_root Matthew Wilcox
2017-11-22 21:06 ` [PATCH 15/62] page cache: Use xa_lock Matthew Wilcox
2017-11-22 21:06 ` [PATCH 16/62] xarray: Replace exceptional entries Matthew Wilcox
2017-11-22 21:06 ` [PATCH 17/62] xarray: Change definition of sibling entries Matthew Wilcox
2017-11-22 21:06 ` [PATCH 18/62] xarray: Add definition of struct xarray Matthew Wilcox
2017-11-22 21:06 ` [PATCH 19/62] xarray: Define struct xa_node Matthew Wilcox
2017-11-22 21:06 ` [PATCH 20/62] xarray: Add xa_load Matthew Wilcox
2017-11-22 21:06 ` [PATCH 21/62] xarray: Add xa_get_tag, xa_set_tag and xa_clear_tag Matthew Wilcox
2017-11-22 21:06 ` [PATCH 22/62] xarray: Add xa_store Matthew Wilcox
2017-11-22 21:07 ` [PATCH 23/62] xarray: Add xa_cmpxchg Matthew Wilcox
2017-11-22 21:07 ` [PATCH 24/62] xarray: Add xa_for_each Matthew Wilcox
2017-11-22 21:07 ` [PATCH 25/62] xarray: Add xa_init Matthew Wilcox
2017-11-22 21:07 ` [PATCH 26/62] xarray: Add xas_for_each_tag Matthew Wilcox
2017-11-22 21:07 ` [PATCH 27/62] xarray: Add xa_get_entries and xa_get_tagged Matthew Wilcox
2017-11-22 21:07 ` [PATCH 28/62] xarray: Add xa_destroy Matthew Wilcox
2017-11-22 21:07 ` [PATCH 29/62] xarray: Add xas_prev_any Matthew Wilcox
2017-11-22 21:07 ` [PATCH 30/62] xarray: Add xas_find_any / xas_next_any Matthew Wilcox
2017-11-22 21:07 ` [PATCH 31/62] Convert IDR to use xarray Matthew Wilcox
2017-11-22 21:07 ` [PATCH 32/62] ida: Convert to using xarray Matthew Wilcox
2017-11-22 21:07 ` [PATCH 33/62] page cache: Convert page_cache_next_hole to XArray Matthew Wilcox
2017-11-22 21:07 ` [PATCH 34/62] page cache: Use xarray for adding pages Matthew Wilcox
2017-11-22 21:07 ` [PATCH 35/62] page cache: Convert page_cache_tree_delete to xarray Matthew Wilcox
2017-11-22 21:07 ` [PATCH 36/62] page cache: Convert find_get_entry " Matthew Wilcox
2017-11-22 21:07 ` [PATCH 37/62] shmem: Convert replace " Matthew Wilcox
2017-11-22 21:07 ` [PATCH 38/62] shmem: Convert shmem_confirm_swap to XArray Matthew Wilcox
2017-11-22 21:07 ` [PATCH 39/62] shmem: Convert find_swap_entry " Matthew Wilcox
2017-11-22 21:07 ` [PATCH 40/62] shmem: Convert shmem_tag_pins " Matthew Wilcox
2017-11-22 21:07 ` [PATCH 41/62] shmem: Convert shmem_wait_for_pins " Matthew Wilcox
2017-11-22 21:07 ` [PATCH 42/62] vmalloc: Convert to xarray Matthew Wilcox
2017-11-22 21:07 ` [PATCH 43/62] brd: Convert to XArray Matthew Wilcox
2017-11-22 21:07 ` [PATCH 44/62] xfs: Convert m_perag_tree " Matthew Wilcox
2017-11-22 21:07 ` [PATCH 45/62] xfs: Convert pag_ici_root " Matthew Wilcox
2017-11-22 21:07 ` [PATCH 46/62] xfs: Convert xfs dquot " Matthew Wilcox
2017-11-22 21:07 ` [PATCH 47/62] xfs: Convert mru cache " Matthew Wilcox
2017-11-22 21:07 ` [PATCH 48/62] block: Remove IDR preloading Matthew Wilcox
2017-11-22 21:07 ` [PATCH 49/62] rxrpc: " Matthew Wilcox
2017-11-22 21:07 ` [PATCH 50/62] cgroup: Remove IDR wrappers Matthew Wilcox
2017-11-22 21:07 ` [PATCH 51/62] dca: Remove idr_preload calls Matthew Wilcox
2017-11-22 21:07 ` [PATCH 52/62] ipc: Remove call to idr_preload Matthew Wilcox
2017-11-22 21:07 ` [PATCH 53/62] irq: " Matthew Wilcox
2017-11-22 21:07 ` [PATCH 54/62] scsi: Remove idr_preload in st driver Matthew Wilcox
2017-11-22 21:07 ` [PATCH 55/62] firewire: Remove call to idr_preload Matthew Wilcox
2017-11-22 21:07 ` [PATCH 56/62] drm: Remove drm_minor_lock and idr_preload Matthew Wilcox
2017-11-22 21:07 ` [PATCH 57/62] drm: Remove drm_syncobj_fd_to_handle Matthew Wilcox
2017-11-22 21:07 ` [PATCH 58/62] drm: Remove qxl driver IDR locks Matthew Wilcox
2017-11-22 21:07 ` [PATCH 59/62] drm: Replace virtio IDRs with IDAs Matthew Wilcox
2017-11-22 21:07 ` [PATCH 60/62] drm: Replace vmwgfx " Matthew Wilcox
2017-11-22 21:07 ` [PATCH 61/62] net: Redesign act_api use of IDR Matthew Wilcox
2017-11-22 21:07 ` [PATCH 62/62] mm: Convert page-writeback to XArray Matthew Wilcox
2017-11-23  1:25 ` [PATCH 00/62] XArray November 2017 Edition Dave Chinner
2017-11-23  2:46   ` Matthew Wilcox
2017-11-24  1:16 ` XArray documentation Matthew Wilcox
2017-11-24  4:30   ` Andreas Dilger
2017-11-24 17:17     ` Matthew Wilcox
2017-11-24 16:50   ` Martin Steigerwald
2017-11-24 17:03     ` Matthew Wilcox
2017-11-24 18:01       ` Martin Steigerwald [this message]
2017-11-24 19:48         ` Shakeel Butt
2017-11-24 19:56           ` Matthew Wilcox
2017-11-24 21:18         ` Matthew Wilcox
2017-11-24 22:02           ` Martin Steigerwald
2017-11-24 22:08             ` Matthew Wilcox

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2627399.jpLCoM7KBo@merkaba \
    --to=martin@lichtvoll.de \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mawilcox@microsoft.com \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).