RE: [PATCH 2/3] x86/mce/AMD: Define a list_head for threshold blocks outside the list

[Ghannam, Yazen]

> -----Original Message-----
> From: Borislav Petkov [mailto:bp@xxxxxxxxx]
> Sent: Tuesday, May 30, 2017 9:57 AM
> To: Ghannam, Yazen <Yazen.Ghannam@xxxxxxx>
> Cc: linux-edac@xxxxxxxxxxxxxxx; Tony Luck <tony.luck@xxxxxxxxx>;
> x86@xxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH 2/3] x86/mce/AMD: Define a list_head for threshold
> blocks outside the list
> 
> On Tue, May 30, 2017 at 12:39:03PM +0000, Ghannam, Yazen wrote:
> > Like I said in the commit message, the list_head needs to be outside
> > the list to access all the elements using list_for_each*. Otherwise,
> > we won't get a reference to the "head" element since we iterate
> > starting from
> > head->next and break when !head.
> 
> I believe the whole hierarchy here is done a bit differently: the list starts at
> threshold_bank->blocks which points to the first threshold block. So when
> iterating, you need to look at threshold_bank->blocks first, which is the first
> element and then traverse the list.
> 

Yeah, I thought about doing this in the THR interrupt handler. But then I thought
it might be better to re-define the head so that we could iterate through all of
the elements without having to check the first and iterate the rest.

> This is basically how the list gets built:
> 
> allocate_threshold_blocks:
> 
> 	...
> 
>         if (per_cpu(threshold_banks, cpu)[bank]->blocks)
>                 list_add(&b->miscj, &per_cpu(threshold_banks, cpu)[bank]->blocks-
> >miscj);
>         else
>                 per_cpu(threshold_banks, cpu)[bank]->blocks = b;
> 
> per_cpu(threshold_banks, cpu)[bank]->blocks is the first element as it points
> to a struct threshold_block and then the ..->blocks->miscj contains any further
> threshold blocks present on this bank and we queue them there if ->blocks is
> not NULL.
> 

Okay, so I'll redo Patch 3 in this set and drop this one. Any comments on Patch 1?

Thanks,
Yazen