Re: [PATCH 2/4] send callback when swap slot is freed

Hugh Dickins
2009-09-18T07:18:01Z

On Fri, 18 Sep 2009, Pekka Enberg wrote:
> On Fri, 2009-09-18 at 04:13 +0530, Nitin Gupta wrote:
> > +EXPORT_SYMBOL_GPL(set_swap_free_notify);
> > +
> > static int swap_entry_free(struct swap_info_struct *p,
> > swp_entry_t ent, int cache)
> > {
> > @@ -585,6 +617,8 @@ static int swap_entry_free(struct swap_info_struct *p,
> > swap_list.next = p - swap_info;
> > nr_swap_pages++;
> > p->inuse_pages--;
> > + if (p->swap_free_notify_fn)
> > + p->swap_free_notify_fn(p->bdev, offset);
> > }
> > if (!swap_count(count))
> > mem_cgroup_uncharge_swap(ent);
>
> OK, this hits core kernel code so we need to CC some more mm/swapfile.c
> people. The set_swap_free_notify() API looks strange to me. Hugh, I
> think you mentioned that you're okay with an explicit hook. Any
> suggestions how to do this cleanly?

No, no better suggestion. I quite see Nitin's point that ramzswap
would benefit significantly from a callback here, though it's not a
place (holding swap_lock) where we'd like to offer a callback at all.

I think I would prefer the naming to make it absolutely clear that
it's a special for ramzswap or compcache, rather than dressing it
up in the grand generality of a swap_free_notify_fn: giving our
hacks fancy names doesn't really make them better.

(Does the bdev matching work out if there are any regular swapfiles
around? I've not checked, might or might not need refinement there.)

Hugh