Re: [PATCH 1/5] block: move discard checks into the ioctl handler

Keith Busch
2024-03-13T01:22:24Z

On Tue, Mar 12, 2024 at 11:31:31PM +0100, Christoph Hellwig wrote:
> On Tue, Mar 12, 2024 at 04:12:54PM -0600, Keith Busch wrote:
> > > + if (!nr_sects)
> > > return -EINVAL;
> > > + if ((sector | nr_sects) & bs_mask)
> > > return -EINVAL;
> > > -
> > > if (start + len > bdev_nr_bytes(bdev))
> > > return -EINVAL;
> >
> > Maybe you want to shift lower bytes out of consideration, but it is
> > different, right? For example, if I call this ioctl with start=5 and
> > len=555, it would return EINVAL, but your change would let it succeed
> > the same as if start=0, len=512.
>
> We did the same before, just down in __blkdev_issue_discard instead of
> in the ioctl handler.

Here's an example program demonstrating the difference:

discard-test.c:
---
#include <stdio.h>
#include <stdint.h>
#include <fcntl.h>
#include <linux/fs.h>
#include <sys/ioctl.h>

int main(int argc, char **argv)
{
uint64_t range[2];
int fd;

if (argc < 2)
return -1;

fd = open(argv[1], O_RDWR);
if (fd < 0)
return fd;

range[0] = 5;
range[1] = 555;
ioctl(fd, BLKDISCARD, &range);
perror("BLKDISCARD");

return 0;
}
--

Before:

# ./discard-test /dev/nvme0n1
BLKDISCARD: Invalid argument

After:

# ./discard-test /dev/nvme0n1
BLKDISCARD: Success