lkml.org 
[lkml]   [2017]   [Apr]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH -mm 3/5] fault-inject: make fail-nth read/write interface symmetric
    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.

    > + len = simple_read_from_buffer(buf, count, ppos, numbuf, len);
    > +
    > + return len;
    > }
    >
    > static const struct file_operations proc_fail_nth_operations = {
    > --
    > 2.7.4
    >

    \
     
     \ /
      Last update: 2017-04-07 22:38    [W:2.473 / U:0.032 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site