Re: [RFC PATCH 5/6] zsmalloc: introduce handle mapping API

[Sergey Senozhatsky <senozhatsky@chromium.org>]

On (25/01/27 21:26), Yosry Ahmed wrote:
> On Mon, Jan 27, 2025 at 04:59:30PM +0900, Sergey Senozhatsky wrote:
> > Introduce new API to map/unmap zsmalloc handle/object.  The key
> > difference is that this API does not impose atomicity restrictions
> > on its users, unlike zs_map_object() which returns with page-faults
> > and preemption disabled
> 
> I think that's not entirely accurate, see below.

Preemption is disabled via zspage-s rwlock_t - zs_map_object() returns
with it being locked and it's being unlocked in zs_unmap_object().  Then
the function disables pagefaults and per-CPU local lock (protects per-CPU
vm-area) additionally disables preemption.

> [..]
> > @@ -1309,12 +1297,14 @@ void *zs_map_object(struct zs_pool *pool, unsigned long handle,
> >  		goto out;
> >  	}
> >  
> > -	/* this object spans two pages */
> > -	zpdescs[0] = zpdesc;
> > -	zpdescs[1] = get_next_zpdesc(zpdesc);
> > -	BUG_ON(!zpdescs[1]);
> > +	ret = area->vm_buf;
> > +	/* disable page faults to match kmap_local_page() return conditions */
> > +	pagefault_disable();
> 
> Is this accurate/necessary? I am looking at kmap_local_page() and I
> don't see it. Maybe that's remnant from the old code using
> kmap_atomic()?

No, this does not look accuare nor neccesary to me.  I asume that's from
a very long time ago, but regardless of that I don't really understand
why that API wants to resemblwe kmap_atomic() (I think that was the
intention).  This interface if expected to be gone so I didn't want
to dig into it and fix it.