On Fri, Aug 8, 2025 at 11:52 AM Pratyush Yadav <pratyush@xxxxxxxxxx> wrote:
>
> On Fri, Aug 08 2025, Pratyush Yadav wrote:
> [...]
> >> @@ -144,14 +144,35 @@ static int __kho_preserve_order(struct kho_mem_track *track, unsigned long pfn,
> >> unsigned int order)
> >> {
> >> struct kho_mem_phys_bits *bits;
> >> - struct kho_mem_phys *physxa;
> >> + struct kho_mem_phys *physxa, *new_physxa;
> >> const unsigned long pfn_high = pfn >> order;
> >>
> >> might_sleep();
> >>
> >> - physxa = xa_load_or_alloc(&track->orders, order, sizeof(*physxa));
> >> - if (IS_ERR(physxa))
> >> - return PTR_ERR(physxa);
> >> + physxa = xa_load(&track->orders, order);
> >> + if (!physxa) {
> >> + new_physxa = kzalloc(sizeof(*physxa), GFP_KERNEL);
> >> + if (!new_physxa)
> >> + return -ENOMEM;
> >> +
> >> + xa_init(&new_physxa->phys_bits);
> >> + physxa = xa_cmpxchg(&track->orders, order, NULL, new_physxa,
> >> + GFP_KERNEL);
> >> + if (xa_is_err(physxa)) {
> >> + int err = xa_err(physxa);
> >> +
> >> + xa_destroy(&new_physxa->phys_bits);
> >> + kfree(new_physxa);
> >> +
> >> + return err;
> >> + }
> >> + if (physxa) {
> >> + xa_destroy(&new_physxa->phys_bits);
> >> + kfree(new_physxa);
> >> + } else {
> >> + physxa = new_physxa;
> >> + }
> >
> > I suppose this could be simplified a bit to:
> >
> > err = xa_err(physxa);
> > if (err || physxa) {
> > xa_destroy(&new_physxa->phys_bits);
> > kfree(new_physxa);
> >
> > if (err)
> > return err;
> > } else {
> > physxa = new_physxa;
> > }
>
> My email client completely messed the whitespace up so this is a bit
> unreadable. Here is what I meant:
>
> err = xa_err(physxa);
> if (err || physxa) {
> xa_destroy(&new_physxa->phys_bits);
> kfree(new_physxa);
>
> if (err)
> return err;
> } else {
> physxa = new_physxa;
> }
>
> [...]
Thanks Pratyush, I will make this simplification change if Andrew does
not take this patch in before the next revision.
Pasha
