Hi Nico! nice to mail with you as always! On Sat, Jan 27, 2024 at 4:51 AM Nicolas Pitre <nico@xxxxxxxxxxx> wrote: > On Sat, 27 Jan 2024, Linus Walleij wrote: > > > Use the scatterlist memory iterator instead of just > > dereferencing virtual memory using sg_virt(). > > This make highmem references work properly. > > > > This driver also has a bug in the PIO sglist handling that > > is fixed as part of the patch: it does not travers the > > list of scatterbuffers: it will just process the first > > item in the list. This is fixed by augmenting the logic > > such that we do not process more than one sgitem > > per IRQ instead of counting down potentially the whole > > length of the request. > > > > We can suspect that the PIO path is quite untested. > > It was tested for sure ... at least by myself ... some 17 years ago ! Hm, on the DMA path the code is taking struct mmc_data .sg_len into account but not on the polled I/O path. But I think sg_len is very often 1, as long as the memory isn't very fragmented so pieces of a file you read/write are all over the place. It needs to be tested under high memory pressure to provoke errors I think. I'm not sure, the block layer people may have some secret testing trick! (I actually have this hardware in a Kirkwood NAS.) > > if (!nodma) > > - dev_dbg(host->dev, "fallback to PIO for data at 0x%p size %d\n", > > - host->pio_ptr, host->pio_size); > > + dev_dbg(host->dev, "fallback to PIO for data\n"); > > Given this message is about telling you why PIO is used despite not > having asked for it, I think it would be nicer to preserve the > equivalent info responsible for this infliction i.e. data->sg->offset > and data->blksz. OK I fix! Yours, Linus Walleij
