From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id ECB41C47079 for ; Wed, 3 Jan 2024 09:18:48 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 7955E8D005B; Wed, 3 Jan 2024 04:18:48 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 71E158D0053; Wed, 3 Jan 2024 04:18:48 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 598728D005B; Wed, 3 Jan 2024 04:18:48 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 41C0A8D0053 for ; Wed, 3 Jan 2024 04:18:48 -0500 (EST) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 1539CA02AE for ; Wed, 3 Jan 2024 09:18:48 +0000 (UTC) X-FDA: 81637449936.05.13A197B Received: from out30-112.freemail.mail.aliyun.com (out30-112.freemail.mail.aliyun.com [115.124.30.112]) by imf01.hostedemail.com (Postfix) with ESMTP id AB6F040018 for ; Wed, 3 Jan 2024 09:18:44 +0000 (UTC) Authentication-Results: imf01.hostedemail.com; dkim=none; spf=pass (imf01.hostedemail.com: domain of yqleng@linux.alibaba.com designates 115.124.30.112 as permitted sender) smtp.mailfrom=yqleng@linux.alibaba.com; dmarc=pass (policy=none) header.from=alibaba.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1704273526; a=rsa-sha256; cv=none; b=JRZeOeHElzrp+VUvKDFC0NZWjDpFaJgnXPMIYsaiSt+omI8dzZ4SZreI7UE0OaU3FM8ABp Dfgb+Kjyz00qNxiHUKjGBsNcNwLHnkT+CAaZmeTQQTlnT5w9QzwRN9Lr7a6icp39fhA0Kp f9tBsZFvvl/8n/QffEWbctFipCvDRxI= ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=none; spf=pass (imf01.hostedemail.com: domain of yqleng@linux.alibaba.com designates 115.124.30.112 as permitted sender) smtp.mailfrom=yqleng@linux.alibaba.com; dmarc=pass (policy=none) header.from=alibaba.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1704273526; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=FN+l6KnogP2/Qm79CJbGbEvUboeGwHqrMG3yPcKHWGI=; b=DSxbBWtbeUVHTmv+PtM/10o9mMhnKeplFTObaDR98q/tDgTnwm/dJJgVLDqr8oziFilzP9 c61Ve4SIdZZIuhtFJ0dsA5R7mPgOUJL2pWXLFxtTzCsMGd7HuHJjY+eppdIggE5rqGTsvY FYLjbaQNQV8POs7hEMLIeODWK0a5Pj4= X-Alimail-AntiSpam:AC=PASS;BC=-1|-1;BR=01201311R501e4;CH=green;DM=||false|;DS=||;FP=0|-1|-1|-1|0|-1|-1|-1;HT=ay29a033018045168;MF=yqleng@linux.alibaba.com;NM=1;PH=DS;RN=26;SR=0;TI=SMTPD_---0VztaQqB_1704273503; Received: from 30.240.100.160(mailfrom:yqleng@linux.alibaba.com fp:SMTPD_---0VztaQqB_1704273503) by smtp.aliyun-inc.com; Wed, 03 Jan 2024 17:18:41 +0800 Content-Type: multipart/alternative; boundary="------------2N25vUcbTkLEtejZNq1Fvbr4" Message-ID: Date: Wed, 3 Jan 2024 17:18:22 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird From: Yiqun Leng Subject: Re: [PATCH v5 33/40] netfs, cachefiles: Pass upper bound length to allow expansion To: Gao Xiang , David Howells Cc: Matthew Wilcox , Marc Dionne , Paulo Alcantara , Shyam Prasad N , Tom Talpey , Dominique Martinet , Eric Van Hensbergen , Ilya Dryomov , Christian Brauner , linux-cachefs@redhat.com, linux-afs@lists.infradead.org, linux-cifs@vger.kernel.org, linux-nfs@vger.kernel.org, ceph-devel@vger.kernel.org, v9fs@lists.linux.dev, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Jeff Layton , Steve French , Jeffle Xu , Jia Zhu , Xin Yin References: <750e8251-ba30-4f53-a17b-73c79e3739ce@linux.alibaba.com> <20231221132400.1601991-1-dhowells@redhat.com> <20231221132400.1601991-34-dhowells@redhat.com> <198744.1704215477@warthog.procyon.org.uk> In-Reply-To: X-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: AB6F040018 X-Stat-Signature: n6g3bm4zfrg6xcsgx6cc969o8oamju38 X-HE-Tag: 1704273524-24386 X-HE-Meta: U2FsdGVkX1+P00gm7dy8CqVclS2q5XU+txeQdBL7djkf8muN67peJfNmQNQ0F/RXlxkFHYWHaVQzUj63RFDSRXBoU9xorVsNpyj1zNfrOYnDuKuNsX5c+8sxm9W5UhEgzriOoJCkAS/d0ynG/SF0f4qtuW7aE5RueMKyVzSFBFp+sC1oRggzCuszcy1cvjwKvCi585hr3asywiTha7cEZNjb4TKpH+lfY2vpGGg8zMXCdotm906GJ6h667kNwJdHXNuLvgc+PiCnC3LY23tF1vvERI2y0coq+deRFsOvvUQ+95kbA406zOCinYSca71+daoZBZan+bgzoDwR1PlIVxPmDaRb+JZcASQTbBRueG7kvn0d2E5LqR37MDbbXi6EF2XJKWrKuFK6xGKaORokyvhaS9X/eK8lBkhND17Tb6N4Fqpb7oTAZiyJKMEFFGbjzokWMZac3sy3G1QOxpyJ8mZOQ4aVKnWuVH13S+TzpMVhFMuN3sKnR6vo6RwCE8xckZjDJFjUmGXi/MWpJfTMO27nPn6uxDMw5dE9zdn9tSs4pwFD7l059UcggL13jHP/ZXCPgx7ozs9ZmYVwmrjyvN/1v/iasdJJNPncby5y7e69P+pVEKBuNr+WUcBcVQSxA6e3BNcFiLTVXP5Cj5zQ04Mgj0n5B3MRdik7pfCrbiXpyvzkzEnJwejHIW+lLS0LMmXPqYiwFSCV9O4J0TcaP6c6jkPHUiRK8tHt9FFfK9k3y4uAOvfi9bW2hZifxvNEb7zU2B2KT0lt+bfSNggsxPGfRXa2KpVcYMyo/joVwCIurt6tcfiWr00WZcugX/E98t9O5bU0SQUIqsKopY1WCDw2bM6XfTlffBn3KdzEvPPpv72TXtPNX4Xm8VjdBpXXIoMOEvnZcAQYK4HPR3UL+5gfHfudQiErFdgXsvtwlGE7EBs/nJB1bnFKsuoJ4c12cRpsmaIeKSUrP0+Raae 9t4jffXJ HIZIaFkB/70gzWrqGp5aE9YJUqeEIzuOwODpLJBjeHLSyYxultFIGbcJCP+J6T2ihH5m/oP47XRmV0uVYR1zIHXkI0mw4xlP1rr6XnYg/G6l4yTVG6O5hQrYlOOFnmARRUipbhXV0s0DRUGHtripUy2dyt86y/+Uc6r7KiJ2IBhQ+RO/OSPNtACmQa6xZ5AgzL6RaaVGFfELyNvbOh00mJxD8xEcVvJ26uajj1m1rSlpP12B15ADHyjPTrlzoPiP/BqBVNFvgjPVAtk0cRd/sBC2uWpVGBja0ncnUj8fLV1amjkrflEPMOhGW1lLU/yPBhqEkUV8C/SgIdnd3XekX0rhn0IqpvXrBE6A/jgZ9eMEGtP3fg6FnVzb+DOqs2k8z8eELyCEwqoALEOWz67E1xG1zVPQtHFwly7mqdydX7NHZ7Gc= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: This is a multi-part message in MIME format. --------------2N25vUcbTkLEtejZNq1Fvbr4 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Tested-by: Yiqun Leng Please get more test details in this link: https://tone.openanolis.cn/ws/nrh4nnio/test_result/106066 Thanks and Regards, Yiqun Leng On 2024/1/3 04:37, Gao Xiang wrote: > > > On 2024/1/3 01:11, David Howells wrote: >> Gao Xiang wrote: >> >>>>        down = start - round_down(start, PAGE_SIZE); >>>>        *_start = start - down; >>>>        *_len = round_up(down + len, PAGE_SIZE); >>>> +    if (down < start || *_len > upper_len) >>>> +        return -ENOBUFS; >>> >>> Sorry for bothering. We just found some strange when testing >>> today-next EROFS over fscache. >>> >>> I'm not sure the meaning of >>>      if (down < start >>> >>> For example, if start is page-aligned, down == 0. >>> >>> so as long as start > 0 and page-aligned, it will return >>> -ENOBUFS.  Does it an intended behavior? >> >> Yeah, I think that's wrong. >> >> Does the attached help? > > (+cc more people for testing) > > Will test and feedback later. > > Thanks, > Gao Xiang > >> >> David >> --- >> >> diff --git a/fs/cachefiles/io.c b/fs/cachefiles/io.c >> index bffffedce4a9..7529b40bc95a 100644 >> --- a/fs/cachefiles/io.c >> +++ b/fs/cachefiles/io.c >> @@ -522,16 +522,22 @@ int __cachefiles_prepare_write(struct >> cachefiles_object *object, >>                      bool no_space_allocated_yet) >>   { >>       struct cachefiles_cache *cache = object->volume->cache; >> -    loff_t start = *_start, pos; >> -    size_t len = *_len, down; >> +    unsigned long long start = *_start, pos; >> +    size_t len = *_len; >>       int ret; >>         /* Round to DIO size */ >> -    down = start - round_down(start, PAGE_SIZE); >> -    *_start = start - down; >> -    *_len = round_up(down + len, PAGE_SIZE); >> -    if (down < start || *_len > upper_len) >> +    start = round_down(*_start, PAGE_SIZE); >> +    if (start != *_start) { >> +        kleave(" = -ENOBUFS [down]"); >> +        return -ENOBUFS; >> +    } >> +    if (*_len > upper_len) { >> +        kleave(" = -ENOBUFS [up]"); >>           return -ENOBUFS; >> +    } >> + >> +    *_len = round_up(len, PAGE_SIZE); >>         /* We need to work out whether there's sufficient disk space >> to perform >>        * the write - but we can skip that check if we have space already >> @@ -542,7 +548,7 @@ int __cachefiles_prepare_write(struct >> cachefiles_object *object, >>         pos = cachefiles_inject_read_error(); >>       if (pos == 0) >> -        pos = vfs_llseek(file, *_start, SEEK_DATA); >> +        pos = vfs_llseek(file, start, SEEK_DATA); >>       if (pos < 0 && pos >= (loff_t)-MAX_ERRNO) { >>           if (pos == -ENXIO) >>               goto check_space; /* Unallocated tail */ >> @@ -550,7 +556,7 @@ int __cachefiles_prepare_write(struct >> cachefiles_object *object, >>                         cachefiles_trace_seek_error); >>           return pos; >>       } >> -    if ((u64)pos >= (u64)*_start + *_len) >> +    if (pos >= start + *_len) >>           goto check_space; /* Unallocated region */ >>         /* We have a block that's at least partially filled - if >> we're low on >> @@ -563,13 +569,13 @@ int __cachefiles_prepare_write(struct >> cachefiles_object *object, >>         pos = cachefiles_inject_read_error(); >>       if (pos == 0) >> -        pos = vfs_llseek(file, *_start, SEEK_HOLE); >> +        pos = vfs_llseek(file, start, SEEK_HOLE); >>       if (pos < 0 && pos >= (loff_t)-MAX_ERRNO) { >>           trace_cachefiles_io_error(object, file_inode(file), pos, >>                         cachefiles_trace_seek_error); >>           return pos; >>       } >> -    if ((u64)pos >= (u64)*_start + *_len) >> +    if (pos >= start + *_len) >>           return 0; /* Fully allocated */ >>         /* Partially allocated, but insufficient space: cull. */ >> @@ -577,7 +583,7 @@ int __cachefiles_prepare_write(struct >> cachefiles_object *object, >>       ret = cachefiles_inject_remove_error(); >>       if (ret == 0) >>           ret = vfs_fallocate(file, FALLOC_FL_PUNCH_HOLE | >> FALLOC_FL_KEEP_SIZE, >> -                    *_start, *_len); >> +                    start, *_len); >>       if (ret < 0) { >>           trace_cachefiles_io_error(object, file_inode(file), ret, >>                         cachefiles_trace_fallocate_error); >> > --------------2N25vUcbTkLEtejZNq1Fvbr4 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 8bit


Tested-by: Yiqun Leng <yqleng@linux.alibaba.com>

Please get more test details in this link: https://tone.openanolis.cn/ws/nrh4nnio/test_result/106066

Thanks and Regards,
Yiqun Leng


On 2024/1/3 04:37, Gao Xiang wrote:


On 2024/1/3 01:11, David Howells wrote:
Gao Xiang <hsiangkao@linux.alibaba.com> wrote:

       down = start - round_down(start, PAGE_SIZE);
       *_start = start - down;
       *_len = round_up(down + len, PAGE_SIZE);
+    if (down < start || *_len > upper_len)
+        return -ENOBUFS;

Sorry for bothering. We just found some strange when testing
today-next EROFS over fscache.

I'm not sure the meaning of
     if (down < start

For example, if start is page-aligned, down == 0.

so as long as start > 0 and page-aligned, it will return
-ENOBUFS.  Does it an intended behavior?

Yeah, I think that's wrong.

Does the attached help?

(+cc more people for testing)

Will test and feedback later.

Thanks,
Gao Xiang


David
---

diff --git a/fs/cachefiles/io.c b/fs/cachefiles/io.c
index bffffedce4a9..7529b40bc95a 100644
--- a/fs/cachefiles/io.c
+++ b/fs/cachefiles/io.c
@@ -522,16 +522,22 @@ int __cachefiles_prepare_write(struct cachefiles_object *object,
                     bool no_space_allocated_yet)
  {
      struct cachefiles_cache *cache = object->volume->cache;
-    loff_t start = *_start, pos;
-    size_t len = *_len, down;
+    unsigned long long start = *_start, pos;
+    size_t len = *_len;
      int ret;
        /* Round to DIO size */
-    down = start - round_down(start, PAGE_SIZE);
-    *_start = start - down;
-    *_len = round_up(down + len, PAGE_SIZE);
-    if (down < start || *_len > upper_len)
+    start = round_down(*_start, PAGE_SIZE);
+    if (start != *_start) {
+        kleave(" = -ENOBUFS [down]");
+        return -ENOBUFS;
+    }
+    if (*_len > upper_len) {
+        kleave(" = -ENOBUFS [up]");
          return -ENOBUFS;
+    }
+
+    *_len = round_up(len, PAGE_SIZE);
        /* We need to work out whether there's sufficient disk space to perform
       * the write - but we can skip that check if we have space already
@@ -542,7 +548,7 @@ int __cachefiles_prepare_write(struct cachefiles_object *object,
        pos = cachefiles_inject_read_error();
      if (pos == 0)
-        pos = vfs_llseek(file, *_start, SEEK_DATA);
+        pos = vfs_llseek(file, start, SEEK_DATA);
      if (pos < 0 && pos >= (loff_t)-MAX_ERRNO) {
          if (pos == -ENXIO)
              goto check_space; /* Unallocated tail */
@@ -550,7 +556,7 @@ int __cachefiles_prepare_write(struct cachefiles_object *object,
                        cachefiles_trace_seek_error);
          return pos;
      }
-    if ((u64)pos >= (u64)*_start + *_len)
+    if (pos >= start + *_len)
          goto check_space; /* Unallocated region */
        /* We have a block that's at least partially filled - if we're low on
@@ -563,13 +569,13 @@ int __cachefiles_prepare_write(struct cachefiles_object *object,
        pos = cachefiles_inject_read_error();
      if (pos == 0)
-        pos = vfs_llseek(file, *_start, SEEK_HOLE);
+        pos = vfs_llseek(file, start, SEEK_HOLE);
      if (pos < 0 && pos >= (loff_t)-MAX_ERRNO) {
          trace_cachefiles_io_error(object, file_inode(file), pos,
                        cachefiles_trace_seek_error);
          return pos;
      }
-    if ((u64)pos >= (u64)*_start + *_len)
+    if (pos >= start + *_len)
          return 0; /* Fully allocated */
        /* Partially allocated, but insufficient space: cull. */
@@ -577,7 +583,7 @@ int __cachefiles_prepare_write(struct cachefiles_object *object,
      ret = cachefiles_inject_remove_error();
      if (ret == 0)
          ret = vfs_fallocate(file, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE,
-                    *_start, *_len);
+                    start, *_len);
      if (ret < 0) {
          trace_cachefiles_io_error(object, file_inode(file), ret,
                        cachefiles_trace_fallocate_error);


--------------2N25vUcbTkLEtejZNq1Fvbr4--