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

Hugh Dickins
2009-09-21T11:07:31Z

On Sat, 19 Sep 2009, Pekka Enberg wrote:
> Nitin Gupta wrote:
> > It is understood that this swap notify callback is bit of a hack. I think
> > we will not gain much trying to beautify this hack. However, I agree with
> > Hugh's suggestion to rename this notify callback related function/variables
> > to make it explicit that its completely ramzswap related. I will send a path
> > that affects these renames as reply to patch 0/4.
>
> I don't quite agree and do think that my approach is a better long-term
> solution. That said, it's Hugh's call, not mine. Hugh?

Sorry, Pekka, I do prefer Nitin's more explicit hackery.

Yours of course looks nicer: but again this method is actually just
for the one single use, and it is "exporting" the swap_info_struct to
the block device, whereas I'd prefer to move in the opposite direction,
making that struct internal to swapfile.c. (I'd have done so already,
but noticed TuxOnIce making use of it, and don't want to make life
awkward there.)

Is the main basis for your disgust at the way that Nitin installs the
callback, that loop down the swap_info_structs? I should point out
that it was I who imposed that on Nitin: before that he was passing a
swap entry (or was it a swap type extracted from a swap entry?
I forget), which was the sole reference to a swp_entry_t in his
driver - I advised a bdev interface.

Would a compromise be to extend the #ifdef CONFIG_HIBERNATION around
swap_type_of() to cover ramzswap too, then Nitin use swap_type_of()
on his bdev to get a swap type to use to install the notifier?

I'm not saying that would be better, haven't even thought through
if it works: I'm just looking for a compromise, whereby you and I
don't keep sending Nitin scurrying off in opposite directions.

Hugh