UBI: Can we handle -EINTR differently in erase/write path???

UBI: Can we handle -EINTR differently in erase/write path???

[Vinit Agnihotri]

Hi heres few code snip from erase_worker(); from wl.c
if (err != -EIO) {
        /*
         * If this is not %-EIO, we have no idea what to do. Scheduling
         * this physical eraseblock for erasure again would cause
         * errors again and again. Well, lets switch to RO mode.
         */
        ubi_ro_mode(ubi);
        return err;
    }

Suppose while erasure in progress & someone pressed "Ctrl+C" then UBI
straight way marks entire device as Read only & its really painful
because then you cant even delete that volume as UBI is read only.
Which I guess its not good way. If use cases are considered it is
highly likely that user can hit "Ctrl+c" , like he want to cancel
erase at that time or something like that. So we can handle -EINTR
differently by following way

if (err != -EIO) {
        /*
         * If this is not %-EIO, we have no idea what to do. Scheduling
         * this physical eraseblock for erasure again would cause
         * errors again and again. Well, lets switch to RO mode.
         */
        if (err == -EINTR) {
            schedule_erase(ubi, e, 0);
            return err;
        }
        else {
            ubi_ro_mode(ubi);
            return err;
        }
    }
i.e. we can again put that perticular peb to erase & can return
without making ubi RO mode.
same can be achieved in write path. If write as interrupted then we
can return error to user & can put that peb back to erase  list.

This is my initial understanding about handling -EINTR, however I
would like to know views of others. If all agrees I can post patch for
same.

Thanks & Regards
Vinit.

Re: UBI: Can we handle -EINTR differently in erase/write path???

[Josh Boyer]

On Fri, 2007-07-13 at 11:37 +0530, Vinit Agnihotri wrote:
> Hi heres few code snip from erase_worker(); from wl.c
> if (err != -EIO) {
>         /*
>          * If this is not %-EIO, we have no idea what to do. Scheduling
>          * this physical eraseblock for erasure again would cause
>          * errors again and again. Well, lets switch to RO mode.
>          */
>         ubi_ro_mode(ubi);
>         return err;
>     }
> 
> Suppose while erasure in progress & someone pressed "Ctrl+C" then UBI

This is the kernel...  the erase_worker function is called from a kernel
thread that doesn't allow signals.  How would it get a "Ctrl+C" event?

> This is my initial understanding about handling -EINTR, however I
> would like to know views of others. If all agrees I can post patch for
> same.

That is how you handle the EINTR errno in userspace.  I think you'd need
to show how -EINTR could show up in this code path in the kernel before
worrying about how to handle it.

josh

Re: UBI: Can we handle -EINTR differently in erase/write path???

[Vinit Agnihotri]

Ok the scenario is like this.
1. User created a volume
2. User issues raw writes on device by using "dd"
3.UBI do not find any preerased block & so it tries to erase 1 block
synchronously
4.When sync_erase is invoked... it waits in wait_event_interruptible()
5. At this moment if user presses ctrl+c & wait_event_interruptible()
return -EINTR
6. On error erase_worker() will mark ubi in Read-only mode.

Soo we need to handle -EINTR differently, as I've faced this situation :)

Thanks & Regards
Vinit.


/13/07, Josh Boyer <jwboyer@linux.vnet.ibm.com> wrote:
> On Fri, 2007-07-13 at 11:37 +0530, Vinit Agnihotri wrote:
> > Hi heres few code snip from erase_worker(); from wl.c
> > if (err != -EIO) {
> >         /*
> >          * If this is not %-EIO, we have no idea what to do. Scheduling
> >          * this physical eraseblock for erasure again would cause
> >          * errors again and again. Well, lets switch to RO mode.
> >          */
> >         ubi_ro_mode(ubi);
> >         return err;
> >     }
> >
> > Suppose while erasure in progress & someone pressed "Ctrl+C" then UBI
>
> This is the kernel...  the erase_worker function is called from a kernel
> thread that doesn't allow signals.  How would it get a "Ctrl+C" event?
>
> > This is my initial understanding about handling -EINTR, however I
> > would like to know views of others. If all agrees I can post patch for
> > same.
>
> That is how you handle the EINTR errno in userspace.  I think you'd need
> to show how -EINTR could show up in this code path in the kernel before
> worrying about how to handle it.
>
> josh
>
>


-- 
I feel free now

Re: UBI: Can we handle -EINTR differently in erase/write path???

[Artem Bityutskiy]

On Fri, 2007-07-13 at 11:37 +0530, Vinit Agnihotri wrote:
> Hi heres few code snip from erase_worker(); from wl.c
> if (err != -EIO) {
>         /*
>          * If this is not %-EIO, we have no idea what to do. Scheduling
>          * this physical eraseblock for erasure again would cause
>          * errors again and again. Well, lets switch to RO mode.
>          */
>         ubi_ro_mode(ubi);
>         return err;
>     }

Yeah, you are right, feel free to send a patch. The only thing I am
curious about where, at what point the task can be interrupted?

-- 
Best regards,
Artem Bityutskiy (Битюцкий Артём)

Re: UBI: Can we handle -EINTR differently in erase/write path???

[Artem Bityutskiy]

On Fri, 2007-07-13 at 11:37 +0530, Vinit Agnihotri wrote:
> Hi heres few code snip from erase_worker(); from wl.c
> if (err != -EIO) {
>         /*
>          * If this is not %-EIO, we have no idea what to do. Scheduling
>          * this physical eraseblock for erasure again would cause
>          * errors again and again. Well, lets switch to RO mode.
>          */
>         ubi_ro_mode(ubi);
>         return err;
>     }
> Suppose while erasure in progress & someone pressed "Ctrl+C" then UBI
> straight way marks entire device as Read only & its really painful
> because then you cant even delete that volume as UBI is read only.
> Which I guess its not good way. If use cases are considered it is
> highly likely that user can hit "Ctrl+c" , like he want to cancel
> erase at that time or something like that. So we can handle -EINTR
> differently by following way

Please, feel free to ignore my silly question and just send the
patch :-)

-- 
Best regards,
Artem Bityutskiy (Битюцкий Артём)

Re: UBI: Can we handle -EINTR differently in erase/write path???

[Vinit Agnihotri]

I'll send the patch by tomorrow with handling for -EINTR in write also.

Thanks & Regards
Vinit.

On 7/17/07, Artem Bityutskiy <dedekind@infradead.org> wrote:
> On Fri, 2007-07-13 at 11:37 +0530, Vinit Agnihotri wrote:
> > Hi heres few code snip from erase_worker(); from wl.c
> > if (err != -EIO) {
> >         /*
> >          * If this is not %-EIO, we have no idea what to do. Scheduling
> >          * this physical eraseblock for erasure again would cause
> >          * errors again and again. Well, lets switch to RO mode.
> >          */
> >         ubi_ro_mode(ubi);
> >         return err;
> >     }
> > Suppose while erasure in progress & someone pressed "Ctrl+C" then UBI
> > straight way marks entire device as Read only & its really painful
> > because then you cant even delete that volume as UBI is read only.
> > Which I guess its not good way. If use cases are considered it is
> > highly likely that user can hit "Ctrl+c" , like he want to cancel
> > erase at that time or something like that. So we can handle -EINTR
> > differently by following way
>
> Please, feel free to ignore my silly question and just send the
> patch :-)
>
> --
> Best regards,
> Artem Bityutskiy (Битюцкий Артём)
>
>


-- 
I feel free now

Re: UBI: Can we handle -EINTR differently in erase/write path???

[Artem Bityutskiy]

On Fri, 2007-07-13 at 11:37 +0530, Vinit Agnihotri wrote:
> Hi heres few code snip from erase_worker(); from wl.c
> if (err != -EIO) {
>         /*
>          * If this is not %-EIO, we have no idea what to do. Scheduling
>          * this physical eraseblock for erasure again would cause
>          * errors again and again. Well, lets switch to RO mode.
>          */
>         ubi_ro_mode(ubi);
>         return err;
>     }
> 
> Suppose while erasure in progress & someone pressed "Ctrl+C" then UBI
> straight way marks entire device as Read only & its really painful
> because then you cant even delete that volume as UBI is read only.
> Which I guess its not good way. If use cases are considered it is
> highly likely that user can hit "Ctrl+c" , like he want to cancel
> erase at that time or something like that. So we can handle -EINTR
> differently by following way

I've fixed this in the erase worker. Please, check.

-- 
Best regards,
Artem Bityutskiy (Битюцкий Артём)