On Thu, Jun 27, 2024 at 8:27 PM David Rientjes <rientjes@xxxxxxxxxx> wrote:
>
> On Wed, 26 Jun 2024, Jiaqi Yan wrote:
>
> > diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> > index 6f5ac334efba..1559e773537f 100644
> > --- a/mm/memory-failure.c
> > +++ b/mm/memory-failure.c
> > @@ -68,6 +68,8 @@ static int sysctl_memory_failure_early_kill __read_mostly;
> >
> > static int sysctl_memory_failure_recovery __read_mostly = 1;
> >
> > +static int sysctl_enable_soft_offline __read_mostly = 1;
> > +
> > atomic_long_t num_poisoned_pages __read_mostly = ATOMIC_LONG_INIT(0);
> >
> > static bool hw_memory_failure __read_mostly = false;
> > @@ -141,6 +143,15 @@ static struct ctl_table memory_failure_table[] = {
> > .extra1 = SYSCTL_ZERO,
> > .extra2 = SYSCTL_ONE,
> > },
> > + {
> > + .procname = "enable_soft_offline",
> > + .data = &sysctl_enable_soft_offline,
> > + .maxlen = sizeof(sysctl_enable_soft_offline),
> > + .mode = 0644,
> > + .proc_handler = proc_dointvec_minmax,
> > + .extra1 = SYSCTL_ZERO,
> > + .extra2 = SYSCTL_ONE,
> > + }
> > };
> >
> > /*
> > @@ -2749,8 +2760,9 @@ static int soft_offline_in_use_page(struct page *page)
> > * @pfn: pfn to soft-offline
> > * @flags: flags. Same as memory_failure().
> > *
> > - * Returns 0 on success
> > - * -EOPNOTSUPP for hwpoison_filter() filtered the error event
> > + * Returns 0 on success,
> > + * -EOPNOTSUPP for hwpoison_filter() filtered the error event, or
> > + * disabled by /proc/sys/vm/enable_soft_offline,
> > * < 0 otherwise negated errno.
> > *
> > * Soft offline a page, by migration or invalidation,
> > @@ -2786,6 +2798,13 @@ int soft_offline_page(unsigned long pfn, int flags)
> > return -EIO;
> > }
> >
> > + if (!sysctl_enable_soft_offline) {
> > + pr_info_once("%#lx: disabled by /proc/sys/vm/enable_soft_offline\n",
> > + pfn);
>
> Any strong reason to include the pfn in the log message?
>
> I'm concerned about allowing a user to deduce the physical mapping for any
> arbitrary page since this is possible to do through MADV_SOFT_OFFLINE and
> I don't think that it adds value, especially if this is pr_info_once().
Agreed printing pfn value doesn't add value. Will get rid of it in v7.
>
> If we remove the pfn, feel free to add
>
> Acked-by: David Rientjes <rientjes@xxxxxxxxxx>
Thanks David!
>
> > + put_ref_page(pfn, flags);
> > + return -EOPNOTSUPP;
> > + }
> > +
> > mutex_lock(&mf_mutex);
> >
> > if (PageHWPoison(page)) {
> > --
> > 2.45.2.741.gdbec12cfda-goog
> >
> >
