2018-12-06 04:53:41

by Long Li

Subject: [PATCH] CIFS: Avoid returning EBUSY to upper layer VFS

From: Long Li <[email protected]>

EBUSY is not handled by VFS, and will be passed to user-mode. This is not
correct as we need to wait for more credits.

This patch also fixes a bug where rsize or wsize is used uninitialized when
the call to server->ops->wait_mtu_credits() fails.

Reported-by: Dan Carpenter <[email protected]>
Signed-off-by: Long Li <[email protected]>
---
fs/cifs/file.c | 31 ++++++-------------------------
1 file changed, 6 insertions(+), 25 deletions(-)

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 74c33d5..c9bc56b 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -2541,14 +2541,13 @@ static int
cifs_resend_wdata(struct cifs_writedata *wdata, struct list_head *wdata_list,
struct cifs_aio_ctx *ctx)
{
- int wait_retry = 0;
unsigned int wsize, credits;
int rc;
struct TCP_Server_Info *server =
tlink_tcon(wdata->cfile->tlink)->ses->server;

/*
- * Try to resend this wdata, waiting for credits up to 3 seconds.
+ * Wait for credits to resend this wdata.
* Note: we are attempting to resend the whole wdata not in segments
*/
do {
@@ -2556,19 +2555,13 @@ cifs_resend_wdata(struct cifs_writedata *wdata, struct list_head *wdata_list,
server, wdata->bytes, &wsize, &credits);

if (rc)
- break;
+ goto out;

if (wsize < wdata->bytes) {
add_credits_and_wake_if(server, credits, 0);
msleep(1000);
- wait_retry++;
}
- } while (wsize < wdata->bytes && wait_retry < 3);
-
- if (wsize < wdata->bytes) {
- rc = -EBUSY;
- goto out;
- }
+ } while (wsize < wdata->bytes);

rc = -EAGAIN;
while (rc == -EAGAIN) {
@@ -3234,14 +3227,13 @@ static int cifs_resend_rdata(struct cifs_readdata *rdata,
struct list_head *rdata_list,
struct cifs_aio_ctx *ctx)
{
- int wait_retry = 0;
unsigned int rsize, credits;
int rc;
struct TCP_Server_Info *server =
tlink_tcon(rdata->cfile->tlink)->ses->server;

/*
- * Try to resend this rdata, waiting for credits up to 3 seconds.
+ * Wait for credits to resend this rdata.
* Note: we are attempting to resend the whole rdata not in segments
*/
do {
@@ -3249,24 +3241,13 @@ static int cifs_resend_rdata(struct cifs_readdata *rdata,
&rsize, &credits);

if (rc)
- break;
+ goto out;

if (rsize < rdata->bytes) {
add_credits_and_wake_if(server, credits, 0);
msleep(1000);
- wait_retry++;
}
- } while (rsize < rdata->bytes && wait_retry < 3);
-
- /*
- * If we can't find enough credits to send this rdata
- * release the rdata and return failure, this will pass
- * whatever I/O amount we have finished to VFS.
- */
- if (rsize < rdata->bytes) {
- rc = -EBUSY;
- goto out;
- }
+ } while (rsize < rdata->bytes);

rc = -EAGAIN;
while (rc == -EAGAIN) {
--
2.7.4

2018-12-06 08:30:26

by Steve French

Subject: Re: [PATCH] CIFS: Avoid returning EBUSY to upper layer VFS

Tentatively pushed to cifs-2.6.git for-next
On Wed, Dec 5, 2018 at 10:52 PM Long Li <[email protected]> wrote:
>
> From: Long Li <[email protected]>
>
> EBUSY is not handled by VFS, and will be passed to user-mode. This is not
> correct as we need to wait for more credits.
>
> This patch also fixes a bug where rsize or wsize is used uninitialized when
> the call to server->ops->wait_mtu_credits() fails.
>
> Reported-by: Dan Carpenter <[email protected]>
> Signed-off-by: Long Li <[email protected]>
> ---
> fs/cifs/file.c | 31 ++++++-------------------------
> 1 file changed, 6 insertions(+), 25 deletions(-)
>
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index 74c33d5..c9bc56b 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -2541,14 +2541,13 @@ static int
> cifs_resend_wdata(struct cifs_writedata *wdata, struct list_head *wdata_list,
> struct cifs_aio_ctx *ctx)
> {
> - int wait_retry = 0;
> unsigned int wsize, credits;
> int rc;
> struct TCP_Server_Info *server =
> tlink_tcon(wdata->cfile->tlink)->ses->server;
>
> /*
> - * Try to resend this wdata, waiting for credits up to 3 seconds.
> + * Wait for credits to resend this wdata.
> * Note: we are attempting to resend the whole wdata not in segments
> */
> do {
> @@ -2556,19 +2555,13 @@ cifs_resend_wdata(struct cifs_writedata *wdata, struct list_head *wdata_list,
> server, wdata->bytes, &wsize, &credits);
>
> if (rc)
> - break;
> + goto out;
>
> if (wsize < wdata->bytes) {
> add_credits_and_wake_if(server, credits, 0);
> msleep(1000);
> - wait_retry++;
> }
> - } while (wsize < wdata->bytes && wait_retry < 3);
> -
> - if (wsize < wdata->bytes) {
> - rc = -EBUSY;
> - goto out;
> - }
> + } while (wsize < wdata->bytes);
>
> rc = -EAGAIN;
> while (rc == -EAGAIN) {
> @@ -3234,14 +3227,13 @@ static int cifs_resend_rdata(struct cifs_readdata *rdata,
> struct list_head *rdata_list,
> struct cifs_aio_ctx *ctx)
> {
> - int wait_retry = 0;
> unsigned int rsize, credits;
> int rc;
> struct TCP_Server_Info *server =
> tlink_tcon(rdata->cfile->tlink)->ses->server;
>
> /*
> - * Try to resend this rdata, waiting for credits up to 3 seconds.
> + * Wait for credits to resend this rdata.
> * Note: we are attempting to resend the whole rdata not in segments
> */
> do {
> @@ -3249,24 +3241,13 @@ static int cifs_resend_rdata(struct cifs_readdata *rdata,
> &rsize, &credits);
>
> if (rc)
> - break;
> + goto out;
>
> if (rsize < rdata->bytes) {
> add_credits_and_wake_if(server, credits, 0);
> msleep(1000);
> - wait_retry++;
> }
> - } while (rsize < rdata->bytes && wait_retry < 3);
> -
> - /*
> - * If we can't find enough credits to send this rdata
> - * release the rdata and return failure, this will pass
> - * whatever I/O amount we have finished to VFS.
> - */
> - if (rsize < rdata->bytes) {
> - rc = -EBUSY;
> - goto out;
> - }
> + } while (rsize < rdata->bytes);
>
> rc = -EAGAIN;
> while (rc == -EAGAIN) {
> --
> 2.7.4
>


--
Thanks,

Steve

2018-12-07 01:49:04

by Pavel Shilovsky

Subject: Re: [PATCH] CIFS: Avoid returning EBUSY to upper layer VFS

чт, 6 дек. 2018 г. в 00:29, Steve French via samba-technical
<[email protected]>:
>
> Tentatively pushed to cifs-2.6.git for-next
> On Wed, Dec 5, 2018 at 10:52 PM Long Li <[email protected]> wrote:
> >
> > From: Long Li <[email protected]>
> >
> > EBUSY is not handled by VFS, and will be passed to user-mode. This is not
> > correct as we need to wait for more credits.
> >
> > This patch also fixes a bug where rsize or wsize is used uninitialized when
> > the call to server->ops->wait_mtu_credits() fails.
> >
> > Reported-by: Dan Carpenter <[email protected]>
> > Signed-off-by: Long Li <[email protected]>
> > ---
> > fs/cifs/file.c | 31 ++++++-------------------------
> > 1 file changed, 6 insertions(+), 25 deletions(-)
> >
> > diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> > index 74c33d5..c9bc56b 100644
> > --- a/fs/cifs/file.c
> > +++ b/fs/cifs/file.c
> > @@ -2541,14 +2541,13 @@ static int
> > cifs_resend_wdata(struct cifs_writedata *wdata, struct list_head *wdata_list,
> > struct cifs_aio_ctx *ctx)
> > {
> > - int wait_retry = 0;
> > unsigned int wsize, credits;
> > int rc;
> > struct TCP_Server_Info *server =
> > tlink_tcon(wdata->cfile->tlink)->ses->server;
> >
> > /*
> > - * Try to resend this wdata, waiting for credits up to 3 seconds.
> > + * Wait for credits to resend this wdata.
> > * Note: we are attempting to resend the whole wdata not in segments
> > */
> > do {
> > @@ -2556,19 +2555,13 @@ cifs_resend_wdata(struct cifs_writedata *wdata, struct list_head *wdata_list,
> > server, wdata->bytes, &wsize, &credits);
> >
> > if (rc)
> > - break;
> > + goto out;
> >
> > if (wsize < wdata->bytes) {
> > add_credits_and_wake_if(server, credits, 0);
> > msleep(1000);
> > - wait_retry++;
> > }
> > - } while (wsize < wdata->bytes && wait_retry < 3);
> > -
> > - if (wsize < wdata->bytes) {
> > - rc = -EBUSY;
> > - goto out;
> > - }
> > + } while (wsize < wdata->bytes);
> >
> > rc = -EAGAIN;
> > while (rc == -EAGAIN) {
> > @@ -3234,14 +3227,13 @@ static int cifs_resend_rdata(struct cifs_readdata *rdata,
> > struct list_head *rdata_list,
> > struct cifs_aio_ctx *ctx)
> > {
> > - int wait_retry = 0;
> > unsigned int rsize, credits;
> > int rc;
> > struct TCP_Server_Info *server =
> > tlink_tcon(rdata->cfile->tlink)->ses->server;
> >
> > /*
> > - * Try to resend this rdata, waiting for credits up to 3 seconds.
> > + * Wait for credits to resend this rdata.
> > * Note: we are attempting to resend the whole rdata not in segments
> > */
> > do {
> > @@ -3249,24 +3241,13 @@ static int cifs_resend_rdata(struct cifs_readdata *rdata,
> > &rsize, &credits);
> >
> > if (rc)
> > - break;
> > + goto out;
> >
> > if (rsize < rdata->bytes) {
> > add_credits_and_wake_if(server, credits, 0);
> > msleep(1000);
> > - wait_retry++;
> > }
> > - } while (rsize < rdata->bytes && wait_retry < 3);
> > -
> > - /*
> > - * If we can't find enough credits to send this rdata
> > - * release the rdata and return failure, this will pass
> > - * whatever I/O amount we have finished to VFS.
> > - */
> > - if (rsize < rdata->bytes) {
> > - rc = -EBUSY;
> > - goto out;
> > - }
> > + } while (rsize < rdata->bytes);
> >
> > rc = -EAGAIN;
> > while (rc == -EAGAIN) {
> > --
> > 2.7.4
> >
>
>
> --
> Thanks,
>
> Steve
>

Reviewed-by: Pavel Shilovsky <[email protected]>

--
Best regards,
Pavel Shilovsky