Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 15 Jun 2022 10:44:31 -0400
From:   Brian Foster <bfoster@redhat.com>
To:     Matthew Wilcox <willy@infradead.org>
Cc:     linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
        ikent@redhat.com, onestero@redhat.com
Subject: Re: [PATCH 3/3] proc: use idr tgid tag hint to iterate pids in
 readdir
Message-ID: <YqnwT2SafZcVaFNO@bfoster>
References: <20220614180949.102914-1-bfoster@redhat.com>
 <20220614180949.102914-4-bfoster@redhat.com>
 <YqniOxl4ACKVp9hM@casper.infradead.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YqniOxl4ACKVp9hM@casper.infradead.org>
Precedence: bulk

On Wed, Jun 15, 2022 at 02:44:27PM +0100, Matthew Wilcox wrote:
> On Tue, Jun 14, 2022 at 02:09:49PM -0400, Brian Foster wrote:
> > +++ b/include/linux/idr.h
> > @@ -185,6 +185,20 @@ static inline bool idr_is_group_lead(struct idr *idr, unsigned long id)
> >  	return radix_tree_tag_get(&idr->idr_rt, id, IDR_TGID);
> >  }
> >  
> > +/*
> > + * Find the next id with a potentially associated TGID task using the internal
> > + * tag. Task association is not guaranteed and must be checked explicitly.
> > + */
> > +static inline struct pid *find_tgid_pid(struct idr *idr, unsigned long id)
> > +{
> > +	struct pid *pid;
> > +
> > +	if (radix_tree_gang_lookup_tag(&idr->idr_rt, (void **) &pid, id, 1,
> > +				       IDR_TGID) != 1)
> > +		return NULL;
> > +	return pid;
> > +}
> 
> The IDR is a generic data structure, and shouldn't know anything about
> PIDs, TGIDs or tasks.
> 

Hm, Ok. So I suppose this interface should probably be split up a bit
more. The idr wrappers can be more generic along the lines of
idr_set_tag(), idr_is_tagged(), idr_get_next_tagged(), etc. This would
use something like an IDR_TAG1 internally (instead of IDR_TGID) to
basically expose support for a single, generic, idr-iternal (but
external radix-tree) tag for idr consumers. (Presumably this would map
directly over to xarray marks per your reply to patch 1). Then, the
find_tgid_pid() bits can be lifted into the pid code along the lines of
find_ge_pid() (along with perhaps some other simple "tgid" wrappers over
the idr tag mechanism).

If that sounds generally reasonable, I'll wait for any feedback on the
functional approach and follow up with something like that in v2..

Brian