Re: [PATCH v1 2/3] mm: memory-failure: Bump memory failure stats to pglist_data

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jan 18, 2023 at 03:05:21PM -0800, Jiaqi Yan wrote:
> On Tue, Jan 17, 2023 at 1:03 AM HORIGUCHI NAOYA(堀口 直也)
> <naoya.horiguchi@xxxxxxx> wrote:
> >
> > On Mon, Jan 16, 2023 at 12:16:13PM -0800, Andrew Morton wrote:
> > > On Mon, 16 Jan 2023 19:39:01 +0000 Jiaqi Yan <jiaqiyan@xxxxxxxxxx> wrote:
> > ...
> > > > @@ -1227,6 +1227,39 @@ static struct page_state error_states[] = {
> > > >  #undef slab
> > > >  #undef reserved
> > > >
> > > > +static void update_per_node_mf_stats(unsigned long pfn,
> > > > +                                enum mf_result result)
> > > > +{
> > > > +   int nid = MAX_NUMNODES;
> > > > +   struct memory_failure_stats *mf_stats = NULL;
> > > > +
> > > > +   nid = pfn_to_nid(pfn);
> > > > +   if (unlikely(nid < 0 || nid >= MAX_NUMNODES)) {
> > > > +           WARN_ONCE(1, "Memory failure: pfn=%#lx, invalid nid=%d", pfn, nid);
> > > > +           return;
> > > > +   }
> > > > +
> > > > +   mf_stats = &NODE_DATA(nid)->mf_stats;
> > > > +   switch (result) {
> > > > +   case MF_IGNORED:
> > > > +           ++mf_stats->pages_ignored;
> > >
> > > What is the locking here, to prevent concurrent increments?
> >
> > mf_mutex should prevent the race.
> 
> Since we are only incrementing these counters, I wonder if it makes
> sense to convert them into atomic_long_t (encapsulated inside
> memory_failure_stats) and do atomic_long_add, instead of introducing a
> struct mutex?

I thought that all callers of action_result() (which is an only caller of
update_per_node_mf_stats()) are called with holding mf_mutex at the
beginning of memory_failure(), so the concurrent increments should not
happen on the new counters.  But writing counters with atomic type/API is
fine to me.

Thanks,
Naoya Horiguchi




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux