On Oct 25, 2022 / 00:42, Chaitanya Kulkarni wrote: > Shinichiro/Yi, > > On 10/23/22 17:50, Shinichiro Kawasaki wrote: > > On Oct 23, 2022 / 23:27, Yi Zhang wrote: > >> On Sat, Oct 22, 2022 at 7:57 AM Shinichiro Kawasaki > >> <shinichiro.kawasaki@xxxxxxx> wrote: > >>> > >>> On Oct 21, 2022 / 21:42, Chaitanya Kulkarni wrote: > >>> > >>> ... > >>> > >>>> I think creating a minimal setup is a part of the testcase and we should > >>>> not change it, unless there is a explicit reason for doing so. > >>> > >>> I see, I find no reason to change the "minimal log size policy". Let's go with > >>> 64MB log size to keep it. > >>> > >>> Yi, would you mind reposting v2 with size=64m? > >> Sure, and before I post it, I want to ask for suggestions about some > >> other code changes: > >> > >> After set log size with 64M, I found nvme/012 nvme/013 will be > >> failed[1], and there was not enough space for fio with size=950m > >> testing. > >> Either [2] or [3] works, which one do you prefer, or do you have some > >> other suggestion for it? Thanks. > > > > Thank you for testing. I guess fio I/O size=950m was chosen subtracting some > > super block and log size from 1GB NVME device size. Now we increase the log > > size, then the I/O size 950m is larger than the usable xfs size, probably. > > > > Chaitania, what' your thought about the fix approach? To keep the "minimal log > > size policy", I guess the approach [3] to reduce fio I/O size to 900m is more > > appropriate, but would like to hear your insight. > > I'm fine with adjusting the size to it can fit with new minimum log > sizes. Thank you for the comment. Then let's go with the size 900m. > > > > > > > From Yi's observation, I found a couple of improvement opportunities which are > > beyond scope of this fix. Here I note them as memorandum (patches are welcome :) > > > > 1) Assuming nvme device size 1GB define in nvme/012 and nvme/013 has relation to > > the fio I/O size 950m defined in common/xfs, these values should be defined > > at single place. Probably we should define both in nvme/012 and nvme/013. > > Agree. > > > > > 2) The fio I/O size 950m is defined in _xfs_run_fio_verify_io() which is called > > from nvme/035. Then, it is implicitly assumed that TEST_DEV for nvme/035 has > > size 1GB (or larger). I found that nvme/035 fails with 512MB nvme device. > > We should fix this by calculating fio I/O size from TEST_DEV size. (Or > > require 1GB nvme device size for the test case.) > > > > Also, agree on this. > > Above two listed fixes should be done as a part of this fix only. > > I'd expect to see a patch series to fix all the issues listed above, > please CC me so I can review this with priority. Thank you for these comments also. Yi already posted the series :) https://lore.kernel.org/linux-block/20221024061319.1133470-1-yi.zhang@xxxxxxxxxx/ -- Shin'ichiro Kawasaki
