Messages in this thread | | | Date | Wed, 12 Jul 2017 13:49:38 -0700 | From | Andrew Morton <> | Subject | Re: [PATCH -mm 3/5] fault-inject: make fail-nth read/write interface symmetric |
| |
On Fri, 7 Apr 2017 22:37:01 +0200 Dmitry Vyukov <dvyukov@google.com> wrote:
> On Thu, Apr 6, 2017 at 4:55 PM, Akinobu Mita <akinobu.mita@gmail.com> wrote: > > The read interface for fail-nth looks a bit odd. Read from this file > > returns "NYYYY..." or "YYYYY..." (this makes me surprise when cat this > > file). Because there is no EOF condition. The first character indicates > > current->fail_nth is zero or not, and then current->fail_nth is reset > > to zero. > > > > Just returning task->fail_nth value is more natural to understand. > > > > Cc: Dmitry Vyukov <dvyukov@google.com> > > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> > > --- > > Documentation/fault-injection/fault-injection.txt | 13 +++++++------ > > fs/proc/base.c | 14 ++++++-------- > > 2 files changed, 13 insertions(+), 14 deletions(-) > > > > diff --git a/Documentation/fault-injection/fault-injection.txt b/Documentation/fault-injection/fault-injection.txt > > index a321905..370ddcb 100644 > > --- a/Documentation/fault-injection/fault-injection.txt > > +++ b/Documentation/fault-injection/fault-injection.txt > > @@ -139,9 +139,9 @@ o proc entries > > - /proc/self/task/<current-tid>/fail-nth: > > > > Write to this file of integer N makes N-th call in the task fail. > > - Read from this file returns a single char 'Y' or 'N' > > - that says if the fault setup with a previous write to this file was > > - injected or not, and disables the fault if it wasn't yet injected. > > + Read from this file returns a integer value. A value of '0' indicates > > + that the fault setup with a previous write to this file was injected. > > + A positive integer N indicates that the fault wasn't yet injected. > > Note that this file enables all types of faults (slab, futex, etc). > > This setting takes precedence over all other generic debugfs settings > > like probability, interval, times, etc. But per-capability settings > > @@ -325,13 +325,14 @@ int main() > > write(fail_nth, buf, strlen(buf)); > > res = socketpair(AF_LOCAL, SOCK_STREAM, 0, fds); > > err = errno; > > - read(fail_nth, buf, 1); > > + pread(fail_nth, buf, sizeof(buf), 0); > > if (res == 0) { > > close(fds[0]); > > close(fds[1]); > > } > > - printf("%d-th fault %c: res=%d/%d\n", i, buf[0], res, err); > > - if (buf[0] != 'Y') > > + printf("%d-th fault %c: res=%d/%d\n", i, atoi(buf) ? 'N' : 'Y', > > + res, err); > > + if (atoi(buf)) > > break; > > } > > return 0; > > diff --git a/fs/proc/base.c b/fs/proc/base.c > > index 42c52e2..9d14215 100644 > > --- a/fs/proc/base.c > > +++ b/fs/proc/base.c > > @@ -1383,7 +1383,8 @@ static ssize_t proc_fail_nth_read(struct file *file, char __user *buf, > > size_t count, loff_t *ppos) > > { > > struct task_struct *task; > > - int err; > > + char numbuf[PROC_NUMBUF]; > > + ssize_t len; > > > > task = get_proc_task(file_inode(file)); > > if (!task) > > @@ -1391,13 +1392,10 @@ static ssize_t proc_fail_nth_read(struct file *file, char __user *buf, > > put_task_struct(task); > > if (task != current) > > return -EPERM; > > - if (count < 1) > > - return -EINVAL; > > - err = put_user((char)(current->fail_nth ? 'N' : 'Y'), buf); > > - if (err) > > - return err; > > - current->fail_nth = 0; > > - return 1; > > + len = snprintf(numbuf, sizeof(numbuf), "%u\n", task->fail_nth); > > If we allow setting this for non current task, then we need to prevent > data races as the task uses task->fail_nth concurrently. Reads then > should use READ_ONCE and writes in fault-inject.c should use > WRITE_ONCE.
This remains unresolved?
| |