Re: [RFC PATCH 1/2] libnvdimm: Add prctl control for disabling synchronous fault support.

[Jan Kara <jack@suse.cz>]

Hi!

On Fri 29-05-20 15:07:31, Aneesh Kumar K.V wrote:
> Thanks Michal. I also missed Jeff in this email thread.

And I think you'll also need some of the sched maintainers for the prctl
bits...

> On 5/29/20 3:03 PM, Michal Suchánek wrote:
> > Adding Jan
> > 
> > On Fri, May 29, 2020 at 11:11:39AM +0530, Aneesh Kumar K.V wrote:
> > > With POWER10, architecture is adding new pmem flush and sync instructions.
> > > The kernel should prevent the usage of MAP_SYNC if applications are not using
> > > the new instructions on newer hardware.
> > > 
> > > This patch adds a prctl option MAP_SYNC_ENABLE that can be used to enable
> > > the usage of MAP_SYNC. The kernel config option is added to allow the user
> > > to control whether MAP_SYNC should be enabled by default or not.
> > > 
> > > Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
...
> > > diff --git a/kernel/fork.c b/kernel/fork.c
> > > index 8c700f881d92..d5a9a363e81e 100644
> > > --- a/kernel/fork.c
> > > +++ b/kernel/fork.c
> > > @@ -963,6 +963,12 @@ __cacheline_aligned_in_smp DEFINE_SPINLOCK(mmlist_lock);
> > >   static unsigned long default_dump_filter = MMF_DUMP_FILTER_DEFAULT;
> > > +#ifdef CONFIG_ARCH_MAP_SYNC_DISABLE
> > > +unsigned long default_map_sync_mask = MMF_DISABLE_MAP_SYNC_MASK;
> > > +#else
> > > +unsigned long default_map_sync_mask = 0;
> > > +#endif
> > > +

I'm not sure CONFIG is really the right approach here. For a distro that would
basically mean to disable MAP_SYNC for all PPC kernels unless application
explicitly uses the right prctl. Shouldn't we rather initialize
default_map_sync_mask on boot based on whether the CPU we run on requires
new flush instructions or not? Otherwise the patch looks sensible.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR