2009-10-05 14:44:50

by Pete

[permalink] [raw]
Subject: [PATCH] fs: added KERNEL-DEBUG flag to printk statments

Signed-off-by: Greg Petr <[email protected]>

Hi,

The following is a trivial patch which adds some KRENEL_DEBUG flags to
the printk. This is my first patch, please review the same.


--- linux-2.6/fs/aio.c 2009-10-05 14:46:11.000000000 +0530
+++ linux-2.6.test/fs/aio.c 2009-10-05 19:46:11.000000000 +0530
@@ -129,7 +129,7 @@ static int aio_setup_ring(struct kioctx
}

info->mmap_size = nr_pages * PAGE_SIZE;
- dprintk("attempting mmap of %lu bytes\n", info->mmap_size);
+ dprintk(KERNEL_DEBUG "attempting mmap of %lu bytes\n",
info->mmap_size);
down_write(&ctx->mm->mmap_sem);
info->mmap_base = do_mmap(NULL, 0, info->mmap_size,
PROT_READ|PROT_WRITE, MAP_ANONYMOUS|MAP_PRIVATE,
@@ -141,7 +141,7 @@ static int aio_setup_ring(struct kioctx
return -EAGAIN;
}

- dprintk("mmap address: 0x%08lx\n", info->mmap_base);
+ dprintk(KERNEL_DEBUG "mmap address: 0x%08lx\n", info->mmap_base);
info->nr_pages = get_user_pages(current, ctx->mm,
info->mmap_base, nr_pages,
1, 0, info->ring_pages, NULL);
@@ -299,7 +299,7 @@ static struct kioctx *ioctx_alloc(unsign
hlist_add_head_rcu(&ctx->list, &mm->ioctx_list);
spin_unlock(&mm->ioctx_lock);

- dprintk("aio: allocated ioctx %p[%ld]: mm=%p mask=0x%x\n",
+ dprintk(KERNEL_DEBUG "aio: allocated ioctx %p[%ld]: mm=%p mask=0x%x
\n",
ctx, ctx->user_id, current->mm, ctx->ring_info.nr);
return ctx;

@@ -312,7 +312,7 @@ out_freectx:
kmem_cache_free(kioctx_cachep, ctx);
ctx = ERR_PTR(-ENOMEM);

- dprintk("aio: error allocating ioctx %p\n", ctx);
+ dprintk(KERNEL_DEBUG "aio: error allocating ioctx %p\n", ctx);
return ctx;
}

@@ -407,7 +407,7 @@ void exit_aio(struct mm_struct *mm)
cancel_work_sync(&ctx->wq.work);

if (1 != atomic_read(&ctx->users))
- printk(KERN_DEBUG
+ printk(KERN_DEBUG,
"exit_aio:ioctx still alive: %d %d %d\n",
atomic_read(&ctx->users), ctx->dead,
ctx->reqs_active);
@@ -952,7 +952,7 @@ int aio_complete(struct kiocb *iocb, lon
event->res = res;
event->res2 = res2;

- dprintk("aio_complete: %p[%lu]: %p: %p %Lx %lx %lx\n",
+ dprintk(KERNEL_DEBUG "aio_complete: %p[%lu]: %p: %p %Lx %lx %lx\n",
ctx, tail, iocb, iocb->ki_obj.user, iocb->ki_user_data,
res, res2);

@@ -1011,7 +1011,7 @@ static int aio_read_evt(struct kioctx *i
int ret = 0;

ring = kmap_atomic(info->ring_pages[0], KM_USER0);
- dprintk("in aio_read_evt h%lu t%lu m%lu\n",
+ dprintk(KERNEL_DEBUG "in aio_read_evt h%lu t%lu m%lu\n",
(unsigned long)ring->head, (unsigned long)ring->tail,
(unsigned long)ring->nr);

@@ -1034,7 +1034,7 @@ static int aio_read_evt(struct kioctx *i

out:
kunmap_atomic(ring, KM_USER0);
- dprintk("leaving aio_read_evt: %d h%lu t%lu\n", ret,
+ dprintk(KERNEL_DEBUG "leaving aio_read_evt: %d h%lu t%lu\n", ret,
(unsigned long)ring->head, (unsigned long)ring->tail);
return ret;
}
@@ -1100,13 +1100,13 @@ retry:
if (unlikely(ret <= 0))
break;

- dprintk("read event: %Lx %Lx %Lx %Lx\n",
+ dprintk(KERNEL_DEBUG "read event: %Lx %Lx %Lx %Lx\n",
ent.data, ent.obj, ent.res, ent.res2);

/* Could we split the check in two? */
ret = -EFAULT;
if (unlikely(copy_to_user(event, &ent, sizeof(ent)))) {
- dprintk("aio: lost an event due to EFAULT.\n");
+ dprintk(KERNEL_DEBUG "aio: lost an event due to EFAULT.\n");
break;
}
ret = 0;
@@ -1176,7 +1176,7 @@ retry:

ret = -EFAULT;
if (unlikely(copy_to_user(event, &ent, sizeof(ent)))) {
- dprintk("aio: lost an event due to EFAULT.\n");
+ dprintk(KERNEL_DEBUG "aio: lost an event due to EFAULT.\n");
break;
}

@@ -1207,7 +1207,7 @@ static void io_destroy(struct kioctx *io
hlist_del_rcu(&ioctx->list);
spin_unlock(&mm->ioctx_lock);

- dprintk("aio_release(%p)\n", ioctx);
+ dprintk(KERNEL_DEBUG "aio_release(%p)\n", ioctx);
if (likely(!was_dead))
put_ioctx(ioctx); /* twice for the list */

@@ -1496,7 +1496,7 @@ static ssize_t aio_setup_iocb(struct kio
kiocb->ki_retry = aio_fsync;
break;
default:
- dprintk("EINVAL: io_submit: no operation provided\n");
+ dprintk(KERNEL_DEBUG "EINVAL: io_submit: no operation provided\n");
ret = -EINVAL;
}

@@ -1581,7 +1581,7 @@ static int io_submit_one(struct kioctx *

ret = put_user(req->ki_key, &user_iocb->aio_key);
if (unlikely(ret)) {
- dprintk("EFAULT: aio_key\n");
+ dprintk(KERNEL_DEBUG "EFAULT: aio_key\n");
goto out_put_req;
}



2009-10-05 15:26:45

by Pete

[permalink] [raw]
Subject: [PATCH] fs: added KERNEL-DEBUG flag to printk statments


Signed-off-by: Greg Petr <[email protected]>

Hi,

PS ignore the previous email.

The following is a trivial patch which adds some KRENEL_DEBUG flags to
the printk. This is my first patch, please review the same.


--- linux-2.6/fs/aio.c 2009-10-05 14:46:11.000000000 +0530
+++ linux-2.6.test/fs/aio.c 2009-10-05 19:46:11.000000000 +0530
@@ -129,7 +129,7 @@ static int aio_setup_ring(struct kioctx
}

info->mmap_size = nr_pages * PAGE_SIZE;
- dprintk("attempting mmap of %lu bytes\n", info->mmap_size);
+ dprintk(KERNEL_DEBUG "attempting mmap of %lu bytes\n",
info->mmap_size);
down_write(&ctx->mm->mmap_sem);
info->mmap_base = do_mmap(NULL, 0, info->mmap_size,
PROT_READ|PROT_WRITE, MAP_ANONYMOUS|MAP_PRIVATE,
@@ -141,7 +141,7 @@ static int aio_setup_ring(struct kioctx
return -EAGAIN;
}

- dprintk("mmap address: 0x%08lx\n", info->mmap_base);
+ dprintk(KERNEL_DEBUG "mmap address: 0x%08lx\n", info->mmap_base);
info->nr_pages = get_user_pages(current, ctx->mm,
info->mmap_base, nr_pages,
1, 0, info->ring_pages, NULL);
@@ -299,7 +299,7 @@ static struct kioctx *ioctx_alloc(unsign
hlist_add_head_rcu(&ctx->list, &mm->ioctx_list);
spin_unlock(&mm->ioctx_lock);

- dprintk("aio: allocated ioctx %p[%ld]: mm=%p mask=0x%x\n",
+ dprintk(KERNEL_DEBUG "aio: allocated ioctx %p[%ld]: mm=%p mask=0x%x
\n",
ctx, ctx->user_id, current->mm, ctx->ring_info.nr);
return ctx;

@@ -312,7 +312,7 @@ out_freectx:
kmem_cache_free(kioctx_cachep, ctx);
ctx = ERR_PTR(-ENOMEM);

- dprintk("aio: error allocating ioctx %p\n", ctx);
+ dprintk(KERNEL_DEBUG "aio: error allocating ioctx %p\n", ctx);
return ctx;
}

@@ -407,7 +407,7 @@ void exit_aio(struct mm_struct *mm)
cancel_work_sync(&ctx->wq.work);

if (1 != atomic_read(&ctx->users))
- printk(KERN_DEBUG
+ printk(KERN_DEBUG,
"exit_aio:ioctx still alive: %d %d %d\n",
atomic_read(&ctx->users), ctx->dead,
ctx->reqs_active);
@@ -952,7 +952,7 @@ int aio_complete(struct kiocb *iocb, lon
event->res = res;
event->res2 = res2;

- dprintk("aio_complete: %p[%lu]: %p: %p %Lx %lx %lx\n",
+ dprintk(KERNEL_DEBUG "aio_complete: %p[%lu]: %p: %p %Lx %lx %lx\n",
ctx, tail, iocb, iocb->ki_obj.user, iocb->ki_user_data,
res, res2);

@@ -1011,7 +1011,7 @@ static int aio_read_evt(struct kioctx *i
int ret = 0;

ring = kmap_atomic(info->ring_pages[0], KM_USER0);
- dprintk("in aio_read_evt h%lu t%lu m%lu\n",
+ dprintk(KERNEL_DEBUG "in aio_read_evt h%lu t%lu m%lu\n",
(unsigned long)ring->head, (unsigned long)ring->tail,
(unsigned long)ring->nr);

@@ -1034,7 +1034,7 @@ static int aio_read_evt(struct kioctx *i

out:
kunmap_atomic(ring, KM_USER0);
- dprintk("leaving aio_read_evt: %d h%lu t%lu\n", ret,
+ dprintk(KERNEL_DEBUG "leaving aio_read_evt: %d h%lu t%lu\n", ret,
(unsigned long)ring->head, (unsigned long)ring->tail);
return ret;
}
@@ -1100,13 +1100,13 @@ retry:
if (unlikely(ret <= 0))
break;

- dprintk("read event: %Lx %Lx %Lx %Lx\n",
+ dprintk(KERNEL_DEBUG "read event: %Lx %Lx %Lx %Lx\n",
ent.data, ent.obj, ent.res, ent.res2);

/* Could we split the check in two? */
ret = -EFAULT;
if (unlikely(copy_to_user(event, &ent, sizeof(ent)))) {
- dprintk("aio: lost an event due to EFAULT.\n");
+ dprintk(KERNEL_DEBUG "aio: lost an event due to EFAULT.\n");
break;
}
ret = 0;
@@ -1176,7 +1176,7 @@ retry:

ret = -EFAULT;
if (unlikely(copy_to_user(event, &ent, sizeof(ent)))) {
- dprintk("aio: lost an event due to EFAULT.\n");
+ dprintk(KERNEL_DEBUG "aio: lost an event due to EFAULT.\n");
break;
}

@@ -1207,7 +1207,7 @@ static void io_destroy(struct kioctx *io
hlist_del_rcu(&ioctx->list);
spin_unlock(&mm->ioctx_lock);

- dprintk("aio_release(%p)\n", ioctx);
+ dprintk(KERNEL_DEBUG "aio_release(%p)\n", ioctx);
if (likely(!was_dead))
put_ioctx(ioctx); /* twice for the list */

@@ -1496,7 +1496,7 @@ static ssize_t aio_setup_iocb(struct kio
kiocb->ki_retry = aio_fsync;
break;
default:
- dprintk("EINVAL: io_submit: no operation provided\n");
+ dprintk(KERNEL_DEBUG "EINVAL: io_submit: no operation provided\n");
ret = -EINVAL;
}

@@ -1581,7 +1581,7 @@ static int io_submit_one(struct kioctx *

ret = put_user(req->ki_key, &user_iocb->aio_key);
if (unlikely(ret)) {
- dprintk("EFAULT: aio_key\n");
+ dprintk(KERNEL_DEBUG "EFAULT: aio_key\n");
goto out_put_req;
}


2009-10-05 17:09:42

by Matthew Wilcox

[permalink] [raw]
Subject: Re: [PATCH] fs: added KERNEL-DEBUG flag to printk statments

On Mon, Oct 05, 2009 at 08:54:45PM +0530, Pete wrote:
> The following is a trivial patch which adds some KRENEL_DEBUG flags to
> the printk. This is my first patch, please review the same.

Did you compile it? I don't find the string KERNEL_DEBUG anywhere in
my kernel.

More generally, if you're going to change these lines at all, it would
be better to convert them to use pr_debug instead of their own custom
debug printer.

Pay special attention to getting DEBUG set in the right place, and
_at least_ compile-test your code, with and without DEBUG enabled.
You should really run your code (again, with and without DEBUG enabled)
and check that it does the right thing.

>
> --- linux-2.6/fs/aio.c 2009-10-05 14:46:11.000000000 +0530
> +++ linux-2.6.test/fs/aio.c 2009-10-05 19:46:11.000000000 +0530
> @@ -129,7 +129,7 @@ static int aio_setup_ring(struct kioctx
> }
>
> info->mmap_size = nr_pages * PAGE_SIZE;
> - dprintk("attempting mmap of %lu bytes\n", info->mmap_size);
> + dprintk(KERNEL_DEBUG "attempting mmap of %lu bytes\n",
> info->mmap_size);
> down_write(&ctx->mm->mmap_sem);
> info->mmap_base = do_mmap(NULL, 0, info->mmap_size,
> PROT_READ|PROT_WRITE, MAP_ANONYMOUS|MAP_PRIVATE,
> @@ -141,7 +141,7 @@ static int aio_setup_ring(struct kioctx
> return -EAGAIN;
> }
>
> - dprintk("mmap address: 0x%08lx\n", info->mmap_base);
> + dprintk(KERNEL_DEBUG "mmap address: 0x%08lx\n", info->mmap_base);
> info->nr_pages = get_user_pages(current, ctx->mm,
> info->mmap_base, nr_pages,
> 1, 0, info->ring_pages, NULL);
> @@ -299,7 +299,7 @@ static struct kioctx *ioctx_alloc(unsign
> hlist_add_head_rcu(&ctx->list, &mm->ioctx_list);
> spin_unlock(&mm->ioctx_lock);
>
> - dprintk("aio: allocated ioctx %p[%ld]: mm=%p mask=0x%x\n",
> + dprintk(KERNEL_DEBUG "aio: allocated ioctx %p[%ld]: mm=%p mask=0x%x
> \n",
> ctx, ctx->user_id, current->mm, ctx->ring_info.nr);
> return ctx;
>
> @@ -312,7 +312,7 @@ out_freectx:
> kmem_cache_free(kioctx_cachep, ctx);
> ctx = ERR_PTR(-ENOMEM);
>
> - dprintk("aio: error allocating ioctx %p\n", ctx);
> + dprintk(KERNEL_DEBUG "aio: error allocating ioctx %p\n", ctx);
> return ctx;
> }
>
> @@ -407,7 +407,7 @@ void exit_aio(struct mm_struct *mm)
> cancel_work_sync(&ctx->wq.work);
>
> if (1 != atomic_read(&ctx->users))
> - printk(KERN_DEBUG
> + printk(KERN_DEBUG,
> "exit_aio:ioctx still alive: %d %d %d\n",
> atomic_read(&ctx->users), ctx->dead,
> ctx->reqs_active);
> @@ -952,7 +952,7 @@ int aio_complete(struct kiocb *iocb, lon
> event->res = res;
> event->res2 = res2;
>
> - dprintk("aio_complete: %p[%lu]: %p: %p %Lx %lx %lx\n",
> + dprintk(KERNEL_DEBUG "aio_complete: %p[%lu]: %p: %p %Lx %lx %lx\n",
> ctx, tail, iocb, iocb->ki_obj.user, iocb->ki_user_data,
> res, res2);
>
> @@ -1011,7 +1011,7 @@ static int aio_read_evt(struct kioctx *i
> int ret = 0;
>
> ring = kmap_atomic(info->ring_pages[0], KM_USER0);
> - dprintk("in aio_read_evt h%lu t%lu m%lu\n",
> + dprintk(KERNEL_DEBUG "in aio_read_evt h%lu t%lu m%lu\n",
> (unsigned long)ring->head, (unsigned long)ring->tail,
> (unsigned long)ring->nr);
>
> @@ -1034,7 +1034,7 @@ static int aio_read_evt(struct kioctx *i
>
> out:
> kunmap_atomic(ring, KM_USER0);
> - dprintk("leaving aio_read_evt: %d h%lu t%lu\n", ret,
> + dprintk(KERNEL_DEBUG "leaving aio_read_evt: %d h%lu t%lu\n", ret,
> (unsigned long)ring->head, (unsigned long)ring->tail);
> return ret;
> }
> @@ -1100,13 +1100,13 @@ retry:
> if (unlikely(ret <= 0))
> break;
>
> - dprintk("read event: %Lx %Lx %Lx %Lx\n",
> + dprintk(KERNEL_DEBUG "read event: %Lx %Lx %Lx %Lx\n",
> ent.data, ent.obj, ent.res, ent.res2);
>
> /* Could we split the check in two? */
> ret = -EFAULT;
> if (unlikely(copy_to_user(event, &ent, sizeof(ent)))) {
> - dprintk("aio: lost an event due to EFAULT.\n");
> + dprintk(KERNEL_DEBUG "aio: lost an event due to EFAULT.\n");
> break;
> }
> ret = 0;
> @@ -1176,7 +1176,7 @@ retry:
>
> ret = -EFAULT;
> if (unlikely(copy_to_user(event, &ent, sizeof(ent)))) {
> - dprintk("aio: lost an event due to EFAULT.\n");
> + dprintk(KERNEL_DEBUG "aio: lost an event due to EFAULT.\n");
> break;
> }
>
> @@ -1207,7 +1207,7 @@ static void io_destroy(struct kioctx *io
> hlist_del_rcu(&ioctx->list);
> spin_unlock(&mm->ioctx_lock);
>
> - dprintk("aio_release(%p)\n", ioctx);
> + dprintk(KERNEL_DEBUG "aio_release(%p)\n", ioctx);
> if (likely(!was_dead))
> put_ioctx(ioctx); /* twice for the list */
>
> @@ -1496,7 +1496,7 @@ static ssize_t aio_setup_iocb(struct kio
> kiocb->ki_retry = aio_fsync;
> break;
> default:
> - dprintk("EINVAL: io_submit: no operation provided\n");
> + dprintk(KERNEL_DEBUG "EINVAL: io_submit: no operation provided\n");
> ret = -EINVAL;
> }
>
> @@ -1581,7 +1581,7 @@ static int io_submit_one(struct kioctx *
>
> ret = put_user(req->ki_key, &user_iocb->aio_key);
> if (unlikely(ret)) {
> - dprintk("EFAULT: aio_key\n");
> + dprintk(KERNEL_DEBUG "EFAULT: aio_key\n");
> goto out_put_req;
> }
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Matthew Wilcox Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."

2009-10-05 17:50:43

by Pete

[permalink] [raw]
Subject: Re: [PATCH] fs: added KERNEL-DEBUG flag to printk statments


> Pay special attention to getting DEBUG set in the right place, and
> _at least_ compile-test your code, with and without DEBUG enabled.
> You should really run your code (again, with and without DEBUG enabled)
> and check that it does the right thing.

ok - Thanks will redo, test and send a more generalized version of the
patch which modifies the macro instead.
-Greg