| Message ID | 20231103151354.110636-1-michael.roth@amd.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] efi/unaccepted: Fix off-by-one when checking for overlapping ranges | expand |
On 11/3/23 16:13, Michael Roth wrote: > When a task needs to accept memory it will scan the accepting_list > to see if any ranges already being processed by other tasks overlap > with its range. Due to an off-by-one in the range comparisons, a task > might falsely determine that an overlapping range is being accepted, > leading to an unnecessary delay before it begins processing the range. > > Fix the off-by-one in the range comparison to prevent this and slightly > improve performance. > > Fixes: 50e782a86c98 ("efi/unaccepted: Fix soft lockups caused by parallel memory acceptance") > Link: https://lore.kernel.org/linux-mm/20231101004523.vseyi5bezgfaht5i@amd.com/T/#me2eceb9906fcae5fe958b3fe88e41f920f8335b6 > Reviewed-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> > Signed-off-by: Michael Roth <michael.roth@amd.com> More justification for introducing a common ranges_overlap() helper somewhere :) Acked-by: Vlastimil Babka <vbabka@suse.cz> > --- > v2: > * Improve commit message terminology (Kirill) > --- > drivers/firmware/efi/unaccepted_memory.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/firmware/efi/unaccepted_memory.c b/drivers/firmware/efi/unaccepted_memory.c > index 135278ddaf62..79fb687bb90f 100644 > --- a/drivers/firmware/efi/unaccepted_memory.c > +++ b/drivers/firmware/efi/unaccepted_memory.c > @@ -100,7 +100,7 @@ void accept_memory(phys_addr_t start, phys_addr_t end) > * overlap on physical address level. > */ > list_for_each_entry(entry, &accepting_list, list) { > - if (entry->end < range.start) > + if (entry->end <= range.start) > continue; > if (entry->start >= range.end) > continue;
On Fri, 3 Nov 2023 at 16:30, Vlastimil Babka <vbabka@suse.cz> wrote: > > On 11/3/23 16:13, Michael Roth wrote: > > When a task needs to accept memory it will scan the accepting_list > > to see if any ranges already being processed by other tasks overlap > > with its range. Due to an off-by-one in the range comparisons, a task > > might falsely determine that an overlapping range is being accepted, > > leading to an unnecessary delay before it begins processing the range. > > > > Fix the off-by-one in the range comparison to prevent this and slightly > > improve performance. > > > > Fixes: 50e782a86c98 ("efi/unaccepted: Fix soft lockups caused by parallel memory acceptance") > > Link: https://lore.kernel.org/linux-mm/20231101004523.vseyi5bezgfaht5i@amd.com/T/#me2eceb9906fcae5fe958b3fe88e41f920f8335b6 > > Reviewed-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> > > Signed-off-by: Michael Roth <michael.roth@amd.com> > > More justification for introducing a common ranges_overlap() helper somewhere :) > > Acked-by: Vlastimil Babka <vbabka@suse.cz> > Thanks, I'll take this as a fix. > > --- > > v2: > > * Improve commit message terminology (Kirill) > > --- > > drivers/firmware/efi/unaccepted_memory.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/firmware/efi/unaccepted_memory.c b/drivers/firmware/efi/unaccepted_memory.c > > index 135278ddaf62..79fb687bb90f 100644 > > --- a/drivers/firmware/efi/unaccepted_memory.c > > +++ b/drivers/firmware/efi/unaccepted_memory.c > > @@ -100,7 +100,7 @@ void accept_memory(phys_addr_t start, phys_addr_t end) > > * overlap on physical address level. > > */ > > list_for_each_entry(entry, &accepting_list, list) { > > - if (entry->end < range.start) > > + if (entry->end <= range.start) > > continue; > > if (entry->start >= range.end) > > continue; >
On 11/8/23 12:21, Ard Biesheuvel wrote: > On Fri, 3 Nov 2023 at 16:30, Vlastimil Babka <vbabka@suse.cz> wrote: >> >> On 11/3/23 16:13, Michael Roth wrote: >> > When a task needs to accept memory it will scan the accepting_list >> > to see if any ranges already being processed by other tasks overlap >> > with its range. Due to an off-by-one in the range comparisons, a task >> > might falsely determine that an overlapping range is being accepted, >> > leading to an unnecessary delay before it begins processing the range. >> > >> > Fix the off-by-one in the range comparison to prevent this and slightly >> > improve performance. >> > >> > Fixes: 50e782a86c98 ("efi/unaccepted: Fix soft lockups caused by parallel memory acceptance") >> > Link: https://lore.kernel.org/linux-mm/20231101004523.vseyi5bezgfaht5i@amd.com/T/#me2eceb9906fcae5fe958b3fe88e41f920f8335b6 >> > Reviewed-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> >> > Signed-off-by: Michael Roth <michael.roth@amd.com> >> >> More justification for introducing a common ranges_overlap() helper somewhere :) >> >> Acked-by: Vlastimil Babka <vbabka@suse.cz> >> > > Thanks, I'll take this as a fix. Ping, can't see it in mainline nor -next? > >> > --- >> > v2: >> > * Improve commit message terminology (Kirill) >> > --- >> > drivers/firmware/efi/unaccepted_memory.c | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> > diff --git a/drivers/firmware/efi/unaccepted_memory.c b/drivers/firmware/efi/unaccepted_memory.c >> > index 135278ddaf62..79fb687bb90f 100644 >> > --- a/drivers/firmware/efi/unaccepted_memory.c >> > +++ b/drivers/firmware/efi/unaccepted_memory.c >> > @@ -100,7 +100,7 @@ void accept_memory(phys_addr_t start, phys_addr_t end) >> > * overlap on physical address level. >> > */ >> > list_for_each_entry(entry, &accepting_list, list) { >> > - if (entry->end < range.start) >> > + if (entry->end <= range.start) >> > continue; >> > if (entry->start >= range.end) >> > continue; >> >
On Tue, 28 Nov 2023 at 12:25, Vlastimil Babka <vbabka@suse.cz> wrote: > > On 11/8/23 12:21, Ard Biesheuvel wrote: > > On Fri, 3 Nov 2023 at 16:30, Vlastimil Babka <vbabka@suse.cz> wrote: > >> > >> On 11/3/23 16:13, Michael Roth wrote: > >> > When a task needs to accept memory it will scan the accepting_list > >> > to see if any ranges already being processed by other tasks overlap > >> > with its range. Due to an off-by-one in the range comparisons, a task > >> > might falsely determine that an overlapping range is being accepted, > >> > leading to an unnecessary delay before it begins processing the range. > >> > > >> > Fix the off-by-one in the range comparison to prevent this and slightly > >> > improve performance. > >> > > >> > Fixes: 50e782a86c98 ("efi/unaccepted: Fix soft lockups caused by parallel memory acceptance") > >> > Link: https://lore.kernel.org/linux-mm/20231101004523.vseyi5bezgfaht5i@amd.com/T/#me2eceb9906fcae5fe958b3fe88e41f920f8335b6 > >> > Reviewed-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> > >> > Signed-off-by: Michael Roth <michael.roth@amd.com> > >> > >> More justification for introducing a common ranges_overlap() helper somewhere :) > >> > >> Acked-by: Vlastimil Babka <vbabka@suse.cz> > >> > > > > Thanks, I'll take this as a fix. > > Ping, can't see it in mainline nor -next? > Apologies - queued up now.
diff --git a/drivers/firmware/efi/unaccepted_memory.c b/drivers/firmware/efi/unaccepted_memory.c index 135278ddaf62..79fb687bb90f 100644 --- a/drivers/firmware/efi/unaccepted_memory.c +++ b/drivers/firmware/efi/unaccepted_memory.c @@ -100,7 +100,7 @@ void accept_memory(phys_addr_t start, phys_addr_t end) * overlap on physical address level. */ list_for_each_entry(entry, &accepting_list, list) { - if (entry->end < range.start) + if (entry->end <= range.start) continue; if (entry->start >= range.end) continue;
When a task needs to accept memory it will scan the accepting_list to see if any ranges already being processed by other tasks overlap with its range. Due to an off-by-one in the range comparisons, a task might falsely determine that an overlapping range is being accepted, leading to an unnecessary delay before it begins processing the range. Fix the off-by-one in the range comparison to prevent this and slightly improve performance. Fixes: 50e782a86c98 ("efi/unaccepted: Fix soft lockups caused by parallel memory acceptance") Link: https://lore.kernel.org/linux-mm/20231101004523.vseyi5bezgfaht5i@amd.com/T/#me2eceb9906fcae5fe958b3fe88e41f920f8335b6 Reviewed-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> Signed-off-by: Michael Roth <michael.roth@amd.com> --- v2: * Improve commit message terminology (Kirill) --- drivers/firmware/efi/unaccepted_memory.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)