On Thu, Oct 29, 2020 at 05:21:04PM +0000, David Woodhouse wrote:
> On Wed, 2020-10-07 at 12:13 -0400, Kevin O'Connor wrote:
> > > > > --- a/src/hw/nvme.c
> > > > > +++ b/src/hw/nvme.c
> > > > > @@ -727,6 +727,22 @@ nvme_cmd_readwrite(struct nvme_namespace *ns, 
> > > > > struct disk_op_s *op, int write)
> > > > >      u16 const max_blocks = NVME_PAGE_SIZE / ns->block_size;
> > > > >      u16 i;
> > > > > 
> > > > > +    /* Split up requests that are larger than the device can handle 
> > > > > */
> > > > > +    if (op->count > ns->max_req_size) {
> > > > > +        u16 count = op->count;
> > > > > +
> > > > > +        /* Handle the first max_req_size elements */
> > > > > +        op->count = ns->max_req_size;
> > > > > +        if (nvme_cmd_readwrite(ns, op, write))
> > > > > +            return res;
> > > > > +
> > > > > +        /* Handle the remainder of the request */
> > > > > +        op->count = count - ns->max_req_size;
> > > > > +        op->lba += ns->max_req_size;
> > > > > +        op->buf_fl += (ns->max_req_size * ns->block_size);
> > > > > +        return nvme_cmd_readwrite(ns, op, write);
> > > > > +    }
> > > > 
> > > > Depending on the disk access, this code could run with a small stack.
> > > > I would avoid recursion.
> > > 
> > > This is tail recursion, which any reasonable compiler converts into
> > > a jmp :). Take a look at the .o file - for me it did become a plain
> > > jump.
> > > 
> > 
> > Okay, that's fine then.
> 
> It's still kind of icky. This used to be a purely iterative function.
> Now for a large unaligned request (which nvme_build_prpl refuses to
> handle) you get a mixture of (mostly tail) recursion and iteration.
> 
> It'll recurse for each max_req_size, then iterate over each page within
> that.
> 
> I'd much rather see just a simple iterative loop. Something like this
> (incremental, completely untested):

FWIW, I agree that a version without recursion (even tail recursion)
would be nice.  If someone wants to test and confirm that, then that
would be great.

-Kevin
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org

Reply via email to