2022-09-15 03:21:38

by Liu Shixin

[permalink] [raw]
Subject: [PATCH v5 2/5] Revert "frontswap: simplify frontswap_register_ops"

This reverts commit f328c1d16e4c764992895ac9c9425cea861b2ca0.

Since we are supported to delay zswap initializaton, we need to invoke
ops->init for the swap device which is already online when register
backend.

Signed-off-by: Liu Shixin <[email protected]>
---
mm/frontswap.c | 42 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 42 insertions(+)

diff --git a/mm/frontswap.c b/mm/frontswap.c
index 279e55b4ed87..aa3a1e3ddf91 100644
--- a/mm/frontswap.c
+++ b/mm/frontswap.c
@@ -96,11 +96,53 @@ static inline void inc_frontswap_invalidates(void) { }
*/
int frontswap_register_ops(const struct frontswap_ops *ops)
{
+ DECLARE_BITMAP(a, MAX_SWAPFILES);
+ DECLARE_BITMAP(b, MAX_SWAPFILES);
+ struct swap_info_struct *si;
+ unsigned int i;
+
if (frontswap_ops)
return -EINVAL;

+ bitmap_zero(a, MAX_SWAPFILES);
+ bitmap_zero(b, MAX_SWAPFILES);
+
+ spin_lock(&swap_lock);
+ plist_for_each_entry(si, &swap_active_head, list) {
+ if (!WARN_ON(!si->frontswap_map))
+ __set_bit(si->type, a);
+ }
+ spin_unlock(&swap_lock);
+
+ /* the new ops needs to know the currently active swap devices */
+ for_each_set_bit(i, a, MAX_SWAPFILES)
+ ops->init(i);
+
frontswap_ops = ops;
static_branch_inc(&frontswap_enabled_key);
+
+ spin_lock(&swap_lock);
+ plist_for_each_entry(si, &swap_active_head, list) {
+ if (si->frontswap_map)
+ __set_bit(si->type, b);
+ }
+ spin_unlock(&swap_lock);
+
+ /*
+ * On the very unlikely chance that a swap device was added or
+ * removed between setting the "a" list bits and the ops init
+ * calls, we re-check and do init or invalidate for any changed
+ * bits.
+ */
+ if (unlikely(!bitmap_equal(a, b, MAX_SWAPFILES))) {
+ for (i = 0; i < MAX_SWAPFILES; i++) {
+ if (!test_bit(i, a) && test_bit(i, b))
+ ops->init(i);
+ else if (test_bit(i, a) && !test_bit(i, b))
+ ops->invalidate_area(i);
+ }
+ }
+
return 0;
}

--
2.25.1


2022-09-20 12:50:30

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH v5 2/5] Revert "frontswap: simplify frontswap_register_ops"

On Thu, Sep 15, 2022 at 11:50:00AM +0800, Liu Shixin wrote:
> This reverts commit f328c1d16e4c764992895ac9c9425cea861b2ca0.
>
> Since we are supported to delay zswap initializaton, we need to invoke
> ops->init for the swap device which is already online when register
> backend.

Why do we "have" to do it. Retroactively supporting functionality on
previously enabled swap devices seems rather odd, and the amount of
cruft added for it here absolutely does not seem to be worth it.

2022-09-21 02:01:36

by Liu Shixin

[permalink] [raw]
Subject: Re: [PATCH v5 2/5] Revert "frontswap: simplify frontswap_register_ops"

On 2022/9/20 20:13, Christoph Hellwig wrote:

> On Thu, Sep 15, 2022 at 11:50:00AM +0800, Liu Shixin wrote:
>> This reverts commit f328c1d16e4c764992895ac9c9425cea861b2ca0.
>>
>> Since we are supported to delay zswap initializaton, we need to invoke
>> ops->init for the swap device which is already online when register
>> backend.
> Why do we "have" to do it. Retroactively supporting functionality on
> previously enabled swap devices seems rather odd, and the amount of
> cruft added for it here absolutely does not seem to be worth it.
Hi Christoph,

This revert makes code complicated, but I think it's necessary. When enable zswap,
I expect it to work for all swap devices as much as possible. In this way, user can enable
zswap and swap device in any order. Since we can enable zswap on previously swap
devices, why not support it to get such benifit?

Thanks,
> .
>

2022-09-27 07:56:35

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH v5 2/5] Revert "frontswap: simplify frontswap_register_ops"

On Wed, Sep 21, 2022 at 09:42:41AM +0800, Liu Shixin wrote:
> This revert makes code complicated, but I think it's necessary. When
> enable zswap, I expect it to work for all swap devices as much as
> possible.

But why would expect something to work on a device that has been
swapped on before? That's not usually how things work.

2022-09-27 11:14:35

by Liu Shixin

[permalink] [raw]
Subject: Re: [PATCH v5 2/5] Revert "frontswap: simplify frontswap_register_ops"



On 2022/9/27 15:27, Christoph Hellwig wrote:
> On Wed, Sep 21, 2022 at 09:42:41AM +0800, Liu Shixin wrote:
>> This revert makes code complicated, but I think it's necessary. When
>> enable zswap, I expect it to work for all swap devices as much as
>> possible.
> But why would expect something to work on a device that has been
> swapped on before? That's not usually how things work.
>
> .
If not do this, while some user enable swap device first and then enable zswap,
zswap will not take effect. The user need to re-enable the swap device which is
inconvenient.
>

2022-09-27 12:40:40

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH v5 2/5] Revert "frontswap: simplify frontswap_register_ops"

On Tue, Sep 27, 2022 at 07:12:58PM +0800, Liu Shixin wrote:
> If not do this, while some user enable swap device first and then enable zswap,
> zswap will not take effect. The user need to re-enable the swap device which is
> inconvenient.

So load the module before swapping on, just like you'd load any driver
before you expect to use it.