* [PATCH] um: Fix WRITE_ZEROES in the UBD Driver @ 2022-01-25 17:14 Frédéric Danis 2022-01-25 17:56 ` Anton Ivanov 2022-01-26 9:47 ` Anton Ivanov 0 siblings, 2 replies; 9+ messages in thread From: Frédéric Danis @ 2022-01-25 17:14 UTC (permalink / raw) To: linux-um Call to fallocate with FALLOC_FL_PUNCH_HOLE on a device backed by a sparse file can end up by missing data, zeroes data range, if the underlying file is used with a tool like bmaptool which will referenced only used spaces. Signed-off-by: Frédéric Danis <frederic.danis@collabora.com> --- arch/um/drivers/ubd_kern.c | 8 +++++++- arch/um/include/shared/os.h | 1 + arch/um/os-Linux/file.c | 9 +++++++++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c index 69d2d0049a61..b03269faef71 100644 --- a/arch/um/drivers/ubd_kern.c +++ b/arch/um/drivers/ubd_kern.c @@ -1526,13 +1526,19 @@ static void do_io(struct io_thread_req *req, struct io_desc *desc) } break; case REQ_OP_DISCARD: - case REQ_OP_WRITE_ZEROES: n = os_falloc_punch(req->fds[bit], off, len); if (n) { req->error = map_error(-n); return; } break; + case REQ_OP_WRITE_ZEROES: + n = os_falloc_zeroes(req->fds[bit], off, len); + if (n) { + req->error = map_error(-n); + return; + } + break; default: WARN_ON_ONCE(1); req->error = BLK_STS_NOTSUPP; diff --git a/arch/um/include/shared/os.h b/arch/um/include/shared/os.h index 00214059d9ec..fafde1d5416e 100644 --- a/arch/um/include/shared/os.h +++ b/arch/um/include/shared/os.h @@ -168,6 +168,7 @@ extern unsigned os_major(unsigned long long dev); extern unsigned os_minor(unsigned long long dev); extern unsigned long long os_makedev(unsigned major, unsigned minor); extern int os_falloc_punch(int fd, unsigned long long offset, int count); +extern int os_falloc_zeroes(int fd, unsigned long long offset, int count); extern int os_eventfd(unsigned int initval, int flags); extern int os_sendmsg_fds(int fd, const void *buf, unsigned int len, const int *fds, unsigned int fds_num); diff --git a/arch/um/os-Linux/file.c b/arch/um/os-Linux/file.c index e4421dbc4c36..fc4450db59bd 100644 --- a/arch/um/os-Linux/file.c +++ b/arch/um/os-Linux/file.c @@ -625,6 +625,15 @@ int os_falloc_punch(int fd, unsigned long long offset, int len) return n; } +int os_falloc_zeroes(int fd, unsigned long long offset, int len) +{ + int n = fallocate(fd, FALLOC_FL_ZERO_RANGE|FALLOC_FL_KEEP_SIZE, offset, len); + + if (n < 0) + return -errno; + return n; +} + int os_eventfd(unsigned int initval, int flags) { int fd = eventfd(initval, flags); -- 2.25.1 _______________________________________________ linux-um mailing list linux-um@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-um ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] um: Fix WRITE_ZEROES in the UBD Driver 2022-01-25 17:14 [PATCH] um: Fix WRITE_ZEROES in the UBD Driver Frédéric Danis @ 2022-01-25 17:56 ` Anton Ivanov 2022-01-25 18:50 ` Frédéric Danis 2022-01-26 9:47 ` Anton Ivanov 1 sibling, 1 reply; 9+ messages in thread From: Anton Ivanov @ 2022-01-25 17:56 UTC (permalink / raw) To: Frédéric Danis, linux-um On 25/01/2022 17:14, Frédéric Danis wrote: > Call to fallocate with FALLOC_FL_PUNCH_HOLE on a device backed by a sparse > file can end up by missing data, zeroes data range, if the underlying file > is used with a tool like bmaptool which will referenced only used spaces. Can you elaborate on when do you miss data. A file with a hole created using FALLOC_FL_PUNCH_HOLE when reading from the hole should return zeroes. If it returns anything else or if the zeroed range exceeds the one specified in falloc - that is an underlying fs bug. > > Signed-off-by: Frédéric Danis <frederic.danis@collabora.com> > --- > arch/um/drivers/ubd_kern.c | 8 +++++++- > arch/um/include/shared/os.h | 1 + > arch/um/os-Linux/file.c | 9 +++++++++ > 3 files changed, 17 insertions(+), 1 deletion(-) > > diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c > index 69d2d0049a61..b03269faef71 100644 > --- a/arch/um/drivers/ubd_kern.c > +++ b/arch/um/drivers/ubd_kern.c > @@ -1526,13 +1526,19 @@ static void do_io(struct io_thread_req *req, struct io_desc *desc) > } > break; > case REQ_OP_DISCARD: > - case REQ_OP_WRITE_ZEROES: > n = os_falloc_punch(req->fds[bit], off, len); > if (n) { > req->error = map_error(-n); > return; > } > break; > + case REQ_OP_WRITE_ZEROES: > + n = os_falloc_zeroes(req->fds[bit], off, len); > + if (n) { > + req->error = map_error(-n); > + return; > + } > + break; > default: > WARN_ON_ONCE(1); > req->error = BLK_STS_NOTSUPP; > diff --git a/arch/um/include/shared/os.h b/arch/um/include/shared/os.h > index 00214059d9ec..fafde1d5416e 100644 > --- a/arch/um/include/shared/os.h > +++ b/arch/um/include/shared/os.h > @@ -168,6 +168,7 @@ extern unsigned os_major(unsigned long long dev); > extern unsigned os_minor(unsigned long long dev); > extern unsigned long long os_makedev(unsigned major, unsigned minor); > extern int os_falloc_punch(int fd, unsigned long long offset, int count); > +extern int os_falloc_zeroes(int fd, unsigned long long offset, int count); > extern int os_eventfd(unsigned int initval, int flags); > extern int os_sendmsg_fds(int fd, const void *buf, unsigned int len, > const int *fds, unsigned int fds_num); > diff --git a/arch/um/os-Linux/file.c b/arch/um/os-Linux/file.c > index e4421dbc4c36..fc4450db59bd 100644 > --- a/arch/um/os-Linux/file.c > +++ b/arch/um/os-Linux/file.c > @@ -625,6 +625,15 @@ int os_falloc_punch(int fd, unsigned long long offset, int len) > return n; > } > > +int os_falloc_zeroes(int fd, unsigned long long offset, int len) > +{ > + int n = fallocate(fd, FALLOC_FL_ZERO_RANGE|FALLOC_FL_KEEP_SIZE, offset, len); > + > + if (n < 0) > + return -errno; > + return n; > +} > + > int os_eventfd(unsigned int initval, int flags) > { > int fd = eventfd(initval, flags); -- Anton R. Ivanov Cambridgegreys Limited. Registered in England. Company Number 10273661 https://www.cambridgegreys.com/ _______________________________________________ linux-um mailing list linux-um@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-um ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] um: Fix WRITE_ZEROES in the UBD Driver 2022-01-25 17:56 ` Anton Ivanov @ 2022-01-25 18:50 ` Frédéric Danis 2022-01-26 9:47 ` Anton Ivanov 0 siblings, 1 reply; 9+ messages in thread From: Frédéric Danis @ 2022-01-25 18:50 UTC (permalink / raw) To: Anton Ivanov, linux-um Hi Anton, On 25/01/2022 18:56, Anton Ivanov wrote: > On 25/01/2022 17:14, Frédéric Danis wrote: >> Call to fallocate with FALLOC_FL_PUNCH_HOLE on a device backed by a >> sparse >> file can end up by missing data, zeroes data range, if the underlying >> file >> is used with a tool like bmaptool which will referenced only used spaces. > > Can you elaborate on when do you miss data. > > A file with a hole created using FALLOC_FL_PUNCH_HOLE when reading > from the hole should return zeroes. > > If it returns anything else or if the zeroed range exceeds the one > specified in falloc - that is an underlying fs bug. I use bmaptool to create a block map of the used part of a raw system image file, then compress the file and at the end use again bmaptool to flash the file to a SDCard. iiuc fallocate() call with FALLOC_FL_PUNCH_HOLE doesn't "register" the space, as it is zeroed, so when bmaptool creates the block map it doesn't reference the "used holes", and when I flash the SDCard those parts are missing, so the filesystem on the SDCard is corrupted. > >> >> Signed-off-by: Frédéric Danis <frederic.danis@collabora.com> >> --- >> arch/um/drivers/ubd_kern.c | 8 +++++++- >> arch/um/include/shared/os.h | 1 + >> arch/um/os-Linux/file.c | 9 +++++++++ >> 3 files changed, 17 insertions(+), 1 deletion(-) >> >> diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c >> index 69d2d0049a61..b03269faef71 100644 >> --- a/arch/um/drivers/ubd_kern.c >> +++ b/arch/um/drivers/ubd_kern.c >> @@ -1526,13 +1526,19 @@ static void do_io(struct io_thread_req *req, >> struct io_desc *desc) >> } >> break; >> case REQ_OP_DISCARD: >> - case REQ_OP_WRITE_ZEROES: >> n = os_falloc_punch(req->fds[bit], off, len); >> if (n) { >> req->error = map_error(-n); >> return; >> } >> break; >> + case REQ_OP_WRITE_ZEROES: >> + n = os_falloc_zeroes(req->fds[bit], off, len); >> + if (n) { >> + req->error = map_error(-n); >> + return; >> + } >> + break; >> default: >> WARN_ON_ONCE(1); >> req->error = BLK_STS_NOTSUPP; >> diff --git a/arch/um/include/shared/os.h b/arch/um/include/shared/os.h >> index 00214059d9ec..fafde1d5416e 100644 >> --- a/arch/um/include/shared/os.h >> +++ b/arch/um/include/shared/os.h >> @@ -168,6 +168,7 @@ extern unsigned os_major(unsigned long long dev); >> extern unsigned os_minor(unsigned long long dev); >> extern unsigned long long os_makedev(unsigned major, unsigned minor); >> extern int os_falloc_punch(int fd, unsigned long long offset, int >> count); >> +extern int os_falloc_zeroes(int fd, unsigned long long offset, int >> count); >> extern int os_eventfd(unsigned int initval, int flags); >> extern int os_sendmsg_fds(int fd, const void *buf, unsigned int len, >> const int *fds, unsigned int fds_num); >> diff --git a/arch/um/os-Linux/file.c b/arch/um/os-Linux/file.c >> index e4421dbc4c36..fc4450db59bd 100644 >> --- a/arch/um/os-Linux/file.c >> +++ b/arch/um/os-Linux/file.c >> @@ -625,6 +625,15 @@ int os_falloc_punch(int fd, unsigned long long >> offset, int len) >> return n; >> } >> +int os_falloc_zeroes(int fd, unsigned long long offset, int len) >> +{ >> + int n = fallocate(fd, FALLOC_FL_ZERO_RANGE|FALLOC_FL_KEEP_SIZE, >> offset, len); >> + >> + if (n < 0) >> + return -errno; >> + return n; >> +} >> + >> int os_eventfd(unsigned int initval, int flags) >> { >> int fd = eventfd(initval, flags); > > -- Frédéric Danis Senior Software Engineer Collabora Ltd. Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, United Kingdom Registered in England & Wales, no. 5513718 _______________________________________________ linux-um mailing list linux-um@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-um ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] um: Fix WRITE_ZEROES in the UBD Driver 2022-01-25 18:50 ` Frédéric Danis @ 2022-01-26 9:47 ` Anton Ivanov 2022-01-26 10:02 ` Frédéric Danis 0 siblings, 1 reply; 9+ messages in thread From: Anton Ivanov @ 2022-01-26 9:47 UTC (permalink / raw) To: Frédéric Danis, linux-um On 25/01/2022 18:50, Frédéric Danis wrote: > Hi Anton, > > On 25/01/2022 18:56, Anton Ivanov wrote: >> On 25/01/2022 17:14, Frédéric Danis wrote: >>> Call to fallocate with FALLOC_FL_PUNCH_HOLE on a device backed by a sparse >>> file can end up by missing data, zeroes data range, if the underlying file >>> is used with a tool like bmaptool which will referenced only used spaces. >> >> Can you elaborate on when do you miss data. >> >> A file with a hole created using FALLOC_FL_PUNCH_HOLE when reading from the hole should return zeroes. >> >> If it returns anything else or if the zeroed range exceeds the one specified in falloc - that is an underlying fs bug. > > I use bmaptool to create a block map of the used part of a raw system image file, then compress the file and at the end use again bmaptool to flash the file to a SDCard. > > iiuc fallocate() call with FALLOC_FL_PUNCH_HOLE doesn't "register" the space, as it is zeroed, so when bmaptool creates the block map it doesn't reference the "used holes", and when I flash the SDCard those parts are missing, so the filesystem on the SDCard is corrupted. I can see what you are doing and I will OK the patch. However, I still do not understand why it does not work. bmaptool creates a map of the used blocks. When copying, it should create the "missing" as sparse, right? A sparse "hole" when read should be always read as zero. If it is not being read as zero when compressing - bug. Ditto at the next stage when flashing. IMHO there is an underlying bug outside UML here somewhere. A > >> >>> >>> Signed-off-by: Frédéric Danis <frederic.danis@collabora.com> >>> --- >>> arch/um/drivers/ubd_kern.c | 8 +++++++- >>> arch/um/include/shared/os.h | 1 + >>> arch/um/os-Linux/file.c | 9 +++++++++ >>> 3 files changed, 17 insertions(+), 1 deletion(-) >>> >>> diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c >>> index 69d2d0049a61..b03269faef71 100644 >>> --- a/arch/um/drivers/ubd_kern.c >>> +++ b/arch/um/drivers/ubd_kern.c >>> @@ -1526,13 +1526,19 @@ static void do_io(struct io_thread_req *req, struct io_desc *desc) >>> } >>> break; >>> case REQ_OP_DISCARD: >>> - case REQ_OP_WRITE_ZEROES: >>> n = os_falloc_punch(req->fds[bit], off, len); >>> if (n) { >>> req->error = map_error(-n); >>> return; >>> } >>> break; >>> + case REQ_OP_WRITE_ZEROES: >>> + n = os_falloc_zeroes(req->fds[bit], off, len); >>> + if (n) { >>> + req->error = map_error(-n); >>> + return; >>> + } >>> + break; >>> default: >>> WARN_ON_ONCE(1); >>> req->error = BLK_STS_NOTSUPP; >>> diff --git a/arch/um/include/shared/os.h b/arch/um/include/shared/os.h >>> index 00214059d9ec..fafde1d5416e 100644 >>> --- a/arch/um/include/shared/os.h >>> +++ b/arch/um/include/shared/os.h >>> @@ -168,6 +168,7 @@ extern unsigned os_major(unsigned long long dev); >>> extern unsigned os_minor(unsigned long long dev); >>> extern unsigned long long os_makedev(unsigned major, unsigned minor); >>> extern int os_falloc_punch(int fd, unsigned long long offset, int count); >>> +extern int os_falloc_zeroes(int fd, unsigned long long offset, int count); >>> extern int os_eventfd(unsigned int initval, int flags); >>> extern int os_sendmsg_fds(int fd, const void *buf, unsigned int len, >>> const int *fds, unsigned int fds_num); >>> diff --git a/arch/um/os-Linux/file.c b/arch/um/os-Linux/file.c >>> index e4421dbc4c36..fc4450db59bd 100644 >>> --- a/arch/um/os-Linux/file.c >>> +++ b/arch/um/os-Linux/file.c >>> @@ -625,6 +625,15 @@ int os_falloc_punch(int fd, unsigned long long offset, int len) >>> return n; >>> } >>> +int os_falloc_zeroes(int fd, unsigned long long offset, int len) >>> +{ >>> + int n = fallocate(fd, FALLOC_FL_ZERO_RANGE|FALLOC_FL_KEEP_SIZE, offset, len); >>> + >>> + if (n < 0) >>> + return -errno; >>> + return n; >>> +} >>> + >>> int os_eventfd(unsigned int initval, int flags) >>> { >>> int fd = eventfd(initval, flags); >> >> > -- Anton R. Ivanov Cambridgegreys Limited. Registered in England. Company Number 10273661 https://www.cambridgegreys.com/ _______________________________________________ linux-um mailing list linux-um@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-um ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] um: Fix WRITE_ZEROES in the UBD Driver 2022-01-26 9:47 ` Anton Ivanov @ 2022-01-26 10:02 ` Frédéric Danis 2022-01-26 10:35 ` Anton Ivanov 0 siblings, 1 reply; 9+ messages in thread From: Frédéric Danis @ 2022-01-26 10:02 UTC (permalink / raw) To: Anton Ivanov, linux-um Hi Anton, On 26/01/2022 10:47, Anton Ivanov wrote: > > > On 25/01/2022 18:50, Frédéric Danis wrote: >> Hi Anton, >> >> On 25/01/2022 18:56, Anton Ivanov wrote: >>> On 25/01/2022 17:14, Frédéric Danis wrote: >>>> Call to fallocate with FALLOC_FL_PUNCH_HOLE on a device backed by a >>>> sparse >>>> file can end up by missing data, zeroes data range, if the >>>> underlying file >>>> is used with a tool like bmaptool which will referenced only used >>>> spaces. >>> >>> Can you elaborate on when do you miss data. >>> >>> A file with a hole created using FALLOC_FL_PUNCH_HOLE when reading >>> from the hole should return zeroes. >>> >>> If it returns anything else or if the zeroed range exceeds the one >>> specified in falloc - that is an underlying fs bug. >> >> I use bmaptool to create a block map of the used part of a raw system >> image file, then compress the file and at the end use again bmaptool >> to flash the file to a SDCard. >> >> iiuc fallocate() call with FALLOC_FL_PUNCH_HOLE doesn't "register" >> the space, as it is zeroed, so when bmaptool creates the block map it >> doesn't reference the "used holes", and when I flash the SDCard those >> parts are missing, so the filesystem on the SDCard is corrupted. > > I can see what you are doing and I will OK the patch. > > However, I still do not understand why it does not work. > > bmaptool creates a map of the used blocks. > > When copying, it should create the "missing" as sparse, right? It depends: - if you're creating a file, in this case there's no problem as bmaptool will create holes which will be seen as zeroed spaces, - or if you're flashing a device like a SDCard, in this case it only writes the referenced blocks and let untouched the other parts which are certainly not full of zeroes, this is why it needs that intentional zeroed parts are not just holes. I'm not sure to be clear :(, bmaptool is better explained at https://github.com/intel/bmap-tools/ > > A sparse "hole" when read should be always read as zero. If it is not > being read as zero when compressing - bug. Ditto at the next stage > when flashing. Compression is optional, I have the same problem if I use bmaptool to flash the uncompressed file as it will copy to device only the referenced blocks in the block map. > > IMHO there is an underlying bug outside UML here somewhere. > > A Regards, Fred > >> >>> >>>> >>>> Signed-off-by: Frédéric Danis <frederic.danis@collabora.com> >>>> --- >>>> arch/um/drivers/ubd_kern.c | 8 +++++++- >>>> arch/um/include/shared/os.h | 1 + >>>> arch/um/os-Linux/file.c | 9 +++++++++ >>>> 3 files changed, 17 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c >>>> index 69d2d0049a61..b03269faef71 100644 >>>> --- a/arch/um/drivers/ubd_kern.c >>>> +++ b/arch/um/drivers/ubd_kern.c >>>> @@ -1526,13 +1526,19 @@ static void do_io(struct io_thread_req >>>> *req, struct io_desc *desc) >>>> } >>>> break; >>>> case REQ_OP_DISCARD: >>>> - case REQ_OP_WRITE_ZEROES: >>>> n = os_falloc_punch(req->fds[bit], off, len); >>>> if (n) { >>>> req->error = map_error(-n); >>>> return; >>>> } >>>> break; >>>> + case REQ_OP_WRITE_ZEROES: >>>> + n = os_falloc_zeroes(req->fds[bit], off, len); >>>> + if (n) { >>>> + req->error = map_error(-n); >>>> + return; >>>> + } >>>> + break; >>>> default: >>>> WARN_ON_ONCE(1); >>>> req->error = BLK_STS_NOTSUPP; >>>> diff --git a/arch/um/include/shared/os.h b/arch/um/include/shared/os.h >>>> index 00214059d9ec..fafde1d5416e 100644 >>>> --- a/arch/um/include/shared/os.h >>>> +++ b/arch/um/include/shared/os.h >>>> @@ -168,6 +168,7 @@ extern unsigned os_major(unsigned long long dev); >>>> extern unsigned os_minor(unsigned long long dev); >>>> extern unsigned long long os_makedev(unsigned major, unsigned >>>> minor); >>>> extern int os_falloc_punch(int fd, unsigned long long offset, int >>>> count); >>>> +extern int os_falloc_zeroes(int fd, unsigned long long offset, int >>>> count); >>>> extern int os_eventfd(unsigned int initval, int flags); >>>> extern int os_sendmsg_fds(int fd, const void *buf, unsigned int len, >>>> const int *fds, unsigned int fds_num); >>>> diff --git a/arch/um/os-Linux/file.c b/arch/um/os-Linux/file.c >>>> index e4421dbc4c36..fc4450db59bd 100644 >>>> --- a/arch/um/os-Linux/file.c >>>> +++ b/arch/um/os-Linux/file.c >>>> @@ -625,6 +625,15 @@ int os_falloc_punch(int fd, unsigned long long >>>> offset, int len) >>>> return n; >>>> } >>>> +int os_falloc_zeroes(int fd, unsigned long long offset, int len) >>>> +{ >>>> + int n = fallocate(fd, >>>> FALLOC_FL_ZERO_RANGE|FALLOC_FL_KEEP_SIZE, offset, len); >>>> + >>>> + if (n < 0) >>>> + return -errno; >>>> + return n; >>>> +} >>>> + >>>> int os_eventfd(unsigned int initval, int flags) >>>> { >>>> int fd = eventfd(initval, flags); >>> >>> >> > -- Frédéric Danis Senior Software Engineer Collabora Ltd. Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, United Kingdom Registered in England & Wales, no. 5513718 _______________________________________________ linux-um mailing list linux-um@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-um ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] um: Fix WRITE_ZEROES in the UBD Driver 2022-01-26 10:02 ` Frédéric Danis @ 2022-01-26 10:35 ` Anton Ivanov 0 siblings, 0 replies; 9+ messages in thread From: Anton Ivanov @ 2022-01-26 10:35 UTC (permalink / raw) To: Frédéric Danis, linux-um On 26/01/2022 10:02, Frédéric Danis wrote: > Hi Anton, > > On 26/01/2022 10:47, Anton Ivanov wrote: >> >> >> On 25/01/2022 18:50, Frédéric Danis wrote: >>> Hi Anton, >>> >>> On 25/01/2022 18:56, Anton Ivanov wrote: >>>> On 25/01/2022 17:14, Frédéric Danis wrote: >>>>> Call to fallocate with FALLOC_FL_PUNCH_HOLE on a device backed by a sparse >>>>> file can end up by missing data, zeroes data range, if the underlying file >>>>> is used with a tool like bmaptool which will referenced only used spaces. >>>> >>>> Can you elaborate on when do you miss data. >>>> >>>> A file with a hole created using FALLOC_FL_PUNCH_HOLE when reading from the hole should return zeroes. >>>> >>>> If it returns anything else or if the zeroed range exceeds the one specified in falloc - that is an underlying fs bug. >>> >>> I use bmaptool to create a block map of the used part of a raw system image file, then compress the file and at the end use again bmaptool to flash the file to a SDCard. >>> >>> iiuc fallocate() call with FALLOC_FL_PUNCH_HOLE doesn't "register" the space, as it is zeroed, so when bmaptool creates the block map it doesn't reference the "used holes", and when I flash the SDCard those parts are missing, so the filesystem on the SDCard is corrupted. >> >> I can see what you are doing and I will OK the patch. >> >> However, I still do not understand why it does not work. >> >> bmaptool creates a map of the used blocks. >> >> When copying, it should create the "missing" as sparse, right? > > It depends: > - if you're creating a file, in this case there's no problem as bmaptool will create holes which will be seen as zeroed spaces, > - or if you're flashing a device like a SDCard, in this case it only writes the referenced blocks and let untouched the other parts which are certainly not full of zeroes, this is why it needs that intentional zeroed parts are not just holes. OK, I understood it now. > > I'm not sure to be clear :(, bmaptool is better explained at https://github.com/intel/bmap-tools/ > >> >> A sparse "hole" when read should be always read as zero. If it is not being read as zero when compressing - bug. Ditto at the next stage when flashing. > > Compression is optional, I have the same problem if I use bmaptool to flash the uncompressed file as it will copy to device only the referenced blocks in the block map. OK, got it. > >> >> IMHO there is an underlying bug outside UML here somewhere. >> >> A > > Regards, > > Fred >> >>> >>>> >>>>> >>>>> Signed-off-by: Frédéric Danis <frederic.danis@collabora.com> >>>>> --- >>>>> arch/um/drivers/ubd_kern.c | 8 +++++++- >>>>> arch/um/include/shared/os.h | 1 + >>>>> arch/um/os-Linux/file.c | 9 +++++++++ >>>>> 3 files changed, 17 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c >>>>> index 69d2d0049a61..b03269faef71 100644 >>>>> --- a/arch/um/drivers/ubd_kern.c >>>>> +++ b/arch/um/drivers/ubd_kern.c >>>>> @@ -1526,13 +1526,19 @@ static void do_io(struct io_thread_req *req, struct io_desc *desc) >>>>> } >>>>> break; >>>>> case REQ_OP_DISCARD: >>>>> - case REQ_OP_WRITE_ZEROES: >>>>> n = os_falloc_punch(req->fds[bit], off, len); >>>>> if (n) { >>>>> req->error = map_error(-n); >>>>> return; >>>>> } >>>>> break; >>>>> + case REQ_OP_WRITE_ZEROES: >>>>> + n = os_falloc_zeroes(req->fds[bit], off, len); >>>>> + if (n) { >>>>> + req->error = map_error(-n); >>>>> + return; >>>>> + } >>>>> + break; >>>>> default: >>>>> WARN_ON_ONCE(1); >>>>> req->error = BLK_STS_NOTSUPP; >>>>> diff --git a/arch/um/include/shared/os.h b/arch/um/include/shared/os.h >>>>> index 00214059d9ec..fafde1d5416e 100644 >>>>> --- a/arch/um/include/shared/os.h >>>>> +++ b/arch/um/include/shared/os.h >>>>> @@ -168,6 +168,7 @@ extern unsigned os_major(unsigned long long dev); >>>>> extern unsigned os_minor(unsigned long long dev); >>>>> extern unsigned long long os_makedev(unsigned major, unsigned minor); >>>>> extern int os_falloc_punch(int fd, unsigned long long offset, int count); >>>>> +extern int os_falloc_zeroes(int fd, unsigned long long offset, int count); >>>>> extern int os_eventfd(unsigned int initval, int flags); >>>>> extern int os_sendmsg_fds(int fd, const void *buf, unsigned int len, >>>>> const int *fds, unsigned int fds_num); >>>>> diff --git a/arch/um/os-Linux/file.c b/arch/um/os-Linux/file.c >>>>> index e4421dbc4c36..fc4450db59bd 100644 >>>>> --- a/arch/um/os-Linux/file.c >>>>> +++ b/arch/um/os-Linux/file.c >>>>> @@ -625,6 +625,15 @@ int os_falloc_punch(int fd, unsigned long long offset, int len) >>>>> return n; >>>>> } >>>>> +int os_falloc_zeroes(int fd, unsigned long long offset, int len) >>>>> +{ >>>>> + int n = fallocate(fd, FALLOC_FL_ZERO_RANGE|FALLOC_FL_KEEP_SIZE, offset, len); >>>>> + >>>>> + if (n < 0) >>>>> + return -errno; >>>>> + return n; >>>>> +} >>>>> + >>>>> int os_eventfd(unsigned int initval, int flags) >>>>> { >>>>> int fd = eventfd(initval, flags); >>>> >>>> >>> >> > -- Anton R. Ivanov Cambridgegreys Limited. Registered in England. Company Number 10273661 https://www.cambridgegreys.com/ _______________________________________________ linux-um mailing list linux-um@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-um ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] um: Fix WRITE_ZEROES in the UBD Driver 2022-01-25 17:14 [PATCH] um: Fix WRITE_ZEROES in the UBD Driver Frédéric Danis 2022-01-25 17:56 ` Anton Ivanov @ 2022-01-26 9:47 ` Anton Ivanov 2022-02-16 10:00 ` Frédéric Danis 1 sibling, 1 reply; 9+ messages in thread From: Anton Ivanov @ 2022-01-26 9:47 UTC (permalink / raw) To: Frédéric Danis, linux-um On 25/01/2022 17:14, Frédéric Danis wrote: > Call to fallocate with FALLOC_FL_PUNCH_HOLE on a device backed by a sparse > file can end up by missing data, zeroes data range, if the underlying file > is used with a tool like bmaptool which will referenced only used spaces. > > Signed-off-by: Frédéric Danis <frederic.danis@collabora.com> > --- > arch/um/drivers/ubd_kern.c | 8 +++++++- > arch/um/include/shared/os.h | 1 + > arch/um/os-Linux/file.c | 9 +++++++++ > 3 files changed, 17 insertions(+), 1 deletion(-) > > diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c > index 69d2d0049a61..b03269faef71 100644 > --- a/arch/um/drivers/ubd_kern.c > +++ b/arch/um/drivers/ubd_kern.c > @@ -1526,13 +1526,19 @@ static void do_io(struct io_thread_req *req, struct io_desc *desc) > } > break; > case REQ_OP_DISCARD: > - case REQ_OP_WRITE_ZEROES: > n = os_falloc_punch(req->fds[bit], off, len); > if (n) { > req->error = map_error(-n); > return; > } > break; > + case REQ_OP_WRITE_ZEROES: > + n = os_falloc_zeroes(req->fds[bit], off, len); > + if (n) { > + req->error = map_error(-n); > + return; > + } > + break; > default: > WARN_ON_ONCE(1); > req->error = BLK_STS_NOTSUPP; > diff --git a/arch/um/include/shared/os.h b/arch/um/include/shared/os.h > index 00214059d9ec..fafde1d5416e 100644 > --- a/arch/um/include/shared/os.h > +++ b/arch/um/include/shared/os.h > @@ -168,6 +168,7 @@ extern unsigned os_major(unsigned long long dev); > extern unsigned os_minor(unsigned long long dev); > extern unsigned long long os_makedev(unsigned major, unsigned minor); > extern int os_falloc_punch(int fd, unsigned long long offset, int count); > +extern int os_falloc_zeroes(int fd, unsigned long long offset, int count); > extern int os_eventfd(unsigned int initval, int flags); > extern int os_sendmsg_fds(int fd, const void *buf, unsigned int len, > const int *fds, unsigned int fds_num); > diff --git a/arch/um/os-Linux/file.c b/arch/um/os-Linux/file.c > index e4421dbc4c36..fc4450db59bd 100644 > --- a/arch/um/os-Linux/file.c > +++ b/arch/um/os-Linux/file.c > @@ -625,6 +625,15 @@ int os_falloc_punch(int fd, unsigned long long offset, int len) > return n; > } > > +int os_falloc_zeroes(int fd, unsigned long long offset, int len) > +{ > + int n = fallocate(fd, FALLOC_FL_ZERO_RANGE|FALLOC_FL_KEEP_SIZE, offset, len); > + > + if (n < 0) > + return -errno; > + return n; > +} > + > int os_eventfd(unsigned int initval, int flags) > { > int fd = eventfd(initval, flags); > Acked-by: Anton Ivanov <anton.ivanov@cambridgegreys.com> -- Anton R. Ivanov Cambridgegreys Limited. Registered in England. Company Number 10273661 https://www.cambridgegreys.com/ _______________________________________________ linux-um mailing list linux-um@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-um ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] um: Fix WRITE_ZEROES in the UBD Driver 2022-01-26 9:47 ` Anton Ivanov @ 2022-02-16 10:00 ` Frédéric Danis 2022-02-16 17:31 ` Anton Ivanov 0 siblings, 1 reply; 9+ messages in thread From: Frédéric Danis @ 2022-02-16 10:00 UTC (permalink / raw) To: Anton Ivanov, linux-um Hi, On 26/01/2022 10:47, Anton Ivanov wrote: > > > On 25/01/2022 17:14, Frédéric Danis wrote: >> Call to fallocate with FALLOC_FL_PUNCH_HOLE on a device backed by a >> sparse >> file can end up by missing data, zeroes data range, if the underlying >> file >> is used with a tool like bmaptool which will referenced only used >> spaces. >> >> Signed-off-by: Frédéric Danis <frederic.danis@collabora.com> >> --- >> arch/um/drivers/ubd_kern.c | 8 +++++++- >> arch/um/include/shared/os.h | 1 + >> arch/um/os-Linux/file.c | 9 +++++++++ >> 3 files changed, 17 insertions(+), 1 deletion(-) >> >> diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c >> index 69d2d0049a61..b03269faef71 100644 >> --- a/arch/um/drivers/ubd_kern.c >> +++ b/arch/um/drivers/ubd_kern.c >> @@ -1526,13 +1526,19 @@ static void do_io(struct io_thread_req *req, >> struct io_desc *desc) >> } >> break; >> case REQ_OP_DISCARD: >> - case REQ_OP_WRITE_ZEROES: >> n = os_falloc_punch(req->fds[bit], off, len); >> if (n) { >> req->error = map_error(-n); >> return; >> } >> break; >> + case REQ_OP_WRITE_ZEROES: >> + n = os_falloc_zeroes(req->fds[bit], off, len); >> + if (n) { >> + req->error = map_error(-n); >> + return; >> + } >> + break; >> default: >> WARN_ON_ONCE(1); >> req->error = BLK_STS_NOTSUPP; >> diff --git a/arch/um/include/shared/os.h b/arch/um/include/shared/os.h >> index 00214059d9ec..fafde1d5416e 100644 >> --- a/arch/um/include/shared/os.h >> +++ b/arch/um/include/shared/os.h >> @@ -168,6 +168,7 @@ extern unsigned os_major(unsigned long long dev); >> extern unsigned os_minor(unsigned long long dev); >> extern unsigned long long os_makedev(unsigned major, unsigned minor); >> extern int os_falloc_punch(int fd, unsigned long long offset, int >> count); >> +extern int os_falloc_zeroes(int fd, unsigned long long offset, int >> count); >> extern int os_eventfd(unsigned int initval, int flags); >> extern int os_sendmsg_fds(int fd, const void *buf, unsigned int len, >> const int *fds, unsigned int fds_num); >> diff --git a/arch/um/os-Linux/file.c b/arch/um/os-Linux/file.c >> index e4421dbc4c36..fc4450db59bd 100644 >> --- a/arch/um/os-Linux/file.c >> +++ b/arch/um/os-Linux/file.c >> @@ -625,6 +625,15 @@ int os_falloc_punch(int fd, unsigned long long >> offset, int len) >> return n; >> } >> +int os_falloc_zeroes(int fd, unsigned long long offset, int len) >> +{ >> + int n = fallocate(fd, FALLOC_FL_ZERO_RANGE|FALLOC_FL_KEEP_SIZE, >> offset, len); >> + >> + if (n < 0) >> + return -errno; >> + return n; >> +} >> + >> int os_eventfd(unsigned int initval, int flags) >> { >> int fd = eventfd(initval, flags); >> > > Acked-by: Anton Ivanov <anton.ivanov@cambridgegreys.com> > What should be next step to get this merged? Regards, Fred -- Frédéric Danis Senior Software Engineer Collabora Ltd. Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, United Kingdom Registered in England & Wales, no. 5513718 _______________________________________________ linux-um mailing list linux-um@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-um ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] um: Fix WRITE_ZEROES in the UBD Driver 2022-02-16 10:00 ` Frédéric Danis @ 2022-02-16 17:31 ` Anton Ivanov 0 siblings, 0 replies; 9+ messages in thread From: Anton Ivanov @ 2022-02-16 17:31 UTC (permalink / raw) To: Frédéric Danis, linux-um; +Cc: richard On 16/02/2022 10:00, Frédéric Danis wrote: > Hi, > > On 26/01/2022 10:47, Anton Ivanov wrote: >> >> >> On 25/01/2022 17:14, Frédéric Danis wrote: >>> Call to fallocate with FALLOC_FL_PUNCH_HOLE on a device backed by a >>> sparse >>> file can end up by missing data, zeroes data range, if the underlying >>> file >>> is used with a tool like bmaptool which will referenced only used >>> spaces. >>> >>> Signed-off-by: Frédéric Danis <frederic.danis@collabora.com> >>> --- >>> arch/um/drivers/ubd_kern.c | 8 +++++++- >>> arch/um/include/shared/os.h | 1 + >>> arch/um/os-Linux/file.c | 9 +++++++++ >>> 3 files changed, 17 insertions(+), 1 deletion(-) >>> >>> diff --git a/arch/um/drivers/ubd_kern.c b/arch/um/drivers/ubd_kern.c >>> index 69d2d0049a61..b03269faef71 100644 >>> --- a/arch/um/drivers/ubd_kern.c >>> +++ b/arch/um/drivers/ubd_kern.c >>> @@ -1526,13 +1526,19 @@ static void do_io(struct io_thread_req *req, >>> struct io_desc *desc) >>> } >>> break; >>> case REQ_OP_DISCARD: >>> - case REQ_OP_WRITE_ZEROES: >>> n = os_falloc_punch(req->fds[bit], off, len); >>> if (n) { >>> req->error = map_error(-n); >>> return; >>> } >>> break; >>> + case REQ_OP_WRITE_ZEROES: >>> + n = os_falloc_zeroes(req->fds[bit], off, len); >>> + if (n) { >>> + req->error = map_error(-n); >>> + return; >>> + } >>> + break; >>> default: >>> WARN_ON_ONCE(1); >>> req->error = BLK_STS_NOTSUPP; >>> diff --git a/arch/um/include/shared/os.h b/arch/um/include/shared/os.h >>> index 00214059d9ec..fafde1d5416e 100644 >>> --- a/arch/um/include/shared/os.h >>> +++ b/arch/um/include/shared/os.h >>> @@ -168,6 +168,7 @@ extern unsigned os_major(unsigned long long dev); >>> extern unsigned os_minor(unsigned long long dev); >>> extern unsigned long long os_makedev(unsigned major, unsigned minor); >>> extern int os_falloc_punch(int fd, unsigned long long offset, int >>> count); >>> +extern int os_falloc_zeroes(int fd, unsigned long long offset, int >>> count); >>> extern int os_eventfd(unsigned int initval, int flags); >>> extern int os_sendmsg_fds(int fd, const void *buf, unsigned int len, >>> const int *fds, unsigned int fds_num); >>> diff --git a/arch/um/os-Linux/file.c b/arch/um/os-Linux/file.c >>> index e4421dbc4c36..fc4450db59bd 100644 >>> --- a/arch/um/os-Linux/file.c >>> +++ b/arch/um/os-Linux/file.c >>> @@ -625,6 +625,15 @@ int os_falloc_punch(int fd, unsigned long long >>> offset, int len) >>> return n; >>> } >>> +int os_falloc_zeroes(int fd, unsigned long long offset, int len) >>> +{ >>> + int n = fallocate(fd, FALLOC_FL_ZERO_RANGE|FALLOC_FL_KEEP_SIZE, >>> offset, len); >>> + >>> + if (n < 0) >>> + return -errno; >>> + return n; >>> +} >>> + >>> int os_eventfd(unsigned int initval, int flags) >>> { >>> int fd = eventfd(initval, flags); >>> >> >> Acked-by: Anton Ivanov <anton.ivanov@cambridgegreys.com> >> > What should be next step to get this merged? I reviewed it and acked it. Richard decides which on the actual merge. A. > > Regards, > > Fred > -- Anton R. Ivanov Cambridgegreys Limited. Registered in England. Company Number 10273661 https://www.cambridgegreys.com/ _______________________________________________ linux-um mailing list linux-um@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-um ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-02-16 17:51 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-01-25 17:14 [PATCH] um: Fix WRITE_ZEROES in the UBD Driver Frédéric Danis 2022-01-25 17:56 ` Anton Ivanov 2022-01-25 18:50 ` Frédéric Danis 2022-01-26 9:47 ` Anton Ivanov 2022-01-26 10:02 ` Frédéric Danis 2022-01-26 10:35 ` Anton Ivanov 2022-01-26 9:47 ` Anton Ivanov 2022-02-16 10:00 ` Frédéric Danis 2022-02-16 17:31 ` Anton Ivanov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox