On 4/22/24 22:06, Michael S. Tsirkin wrote: > On Tue, Apr 09, 2024 at 09:48:08AM +0800, Hou Tao wrote: >> Hi, >> >> On 4/8/2024 3:45 PM, Michael S. Tsirkin wrote: >>> On Wed, Feb 28, 2024 at 10:41:20PM +0800, Hou Tao wrote: >>>> From: Hou Tao <houtao1@xxxxxxxxxx> >>>> >>>> Hi, >>>> >>>> The patch set aims to fix the warning related to an abnormal size >>>> parameter of kmalloc() in virtiofs. The warning occurred when attempting >>>> to insert a 10MB sized kernel module kept in a virtiofs with cache >>>> disabled. As analyzed in patch #1, the root cause is that the length of >>>> the read buffer is no limited, and the read buffer is passed directly to >>>> virtiofs through out_args[0].value. Therefore patch #1 limits the >>>> length of the read buffer passed to virtiofs by using max_pages. However >>>> it is not enough, because now the maximal value of max_pages is 256. >>>> Consequently, when reading a 10MB-sized kernel module, the length of the >>>> bounce buffer in virtiofs will be 40 + (256 * 4096), and kmalloc will >>>> try to allocate 2MB from memory subsystem. The request for 2MB of >>>> physically contiguous memory significantly stress the memory subsystem >>>> and may fail indefinitely on hosts with fragmented memory. To address >>>> this, patch #2~#5 use scattered pages in a bio_vec to replace the >>>> kmalloc-allocated bounce buffer when the length of the bounce buffer for >>>> KVEC_ITER dio is larger than PAGE_SIZE. The final issue with the >>>> allocation of the bounce buffer and sg array in virtiofs is that >>>> GFP_ATOMIC is used even when the allocation occurs in a kworker context. >>>> Therefore the last patch uses GFP_NOFS for the allocation of both sg >>>> array and bounce buffer when initiated by the kworker. For more details, >>>> please check the individual patches. >>>> >>>> As usual, comments are always welcome. >>>> >>>> Change Log: >>> Bernd should I just merge the patchset as is? >>> It seems to fix a real problem and no one has the >>> time to work on a better fix .... WDYT? >> >> Sorry for the long delay. I am just start to prepare for v3. In v3, I >> plan to avoid the unnecessary memory copy between fuse args and bio_vec. >> Will post it before next week. > > Didn't happen before this week apparently. Hi Michael, sorry for my later reply, I had been totally busy for the last weeks as well. Also I can't decide to merge it - I'm not the official fuse maintainer... >From my point of view, patch 1 is just missing to set the actual limit and then would be clear and easy back-portable bug fix. Not promised, I will try it out if I find a bit time tomorrow. Bernd
