* [PATCH 1/2] ramoops: move spin_lock_init after kmalloc error checking @ 2016-08-30 12:24 Geliang Tang 2016-08-30 12:24 ` [PATCH 2/2] ramoops: use buffer_size() and buffer_start() Geliang Tang 2016-09-08 20:45 ` [PATCH 1/2] ramoops: move spin_lock_init after kmalloc error checking Kees Cook 0 siblings, 2 replies; 4+ messages in thread From: Geliang Tang @ 2016-08-30 12:24 UTC (permalink / raw) To: Anton Vorontsov, Colin Cross, Kees Cook, Tony Luck Cc: Geliang Tang, linux-kernel If cxt->pstore.buf allocated failed, no need to initialize cxt->pstore.buf_lock. So this patch moves spin_lock_init() after the error checking. Signed-off-by: Geliang Tang <geliangtang@gmail.com> --- fs/pstore/ram.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index 7a034d6..ec1c9e5 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -608,12 +608,12 @@ static int ramoops_probe(struct platform_device *pdev) cxt->pstore.bufsize = 1024; /* LOG_LINE_MAX */ cxt->pstore.bufsize = max(cxt->record_size, cxt->pstore.bufsize); cxt->pstore.buf = kmalloc(cxt->pstore.bufsize, GFP_KERNEL); - spin_lock_init(&cxt->pstore.buf_lock); if (!cxt->pstore.buf) { pr_err("cannot allocate pstore buffer\n"); err = -ENOMEM; goto fail_clear; } + spin_lock_init(&cxt->pstore.buf_lock); err = pstore_register(&cxt->pstore); if (err) { -- 2.7.4 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] ramoops: use buffer_size() and buffer_start() 2016-08-30 12:24 [PATCH 1/2] ramoops: move spin_lock_init after kmalloc error checking Geliang Tang @ 2016-08-30 12:24 ` Geliang Tang 2016-09-08 20:44 ` Kees Cook 2016-09-08 20:45 ` [PATCH 1/2] ramoops: move spin_lock_init after kmalloc error checking Kees Cook 1 sibling, 1 reply; 4+ messages in thread From: Geliang Tang @ 2016-08-30 12:24 UTC (permalink / raw) To: Anton Vorontsov, Colin Cross, Kees Cook, Tony Luck Cc: Geliang Tang, linux-kernel Since buffer_size() and buffer_start() have been defined in ram_core.c, use them instead of open-coding. Signed-off-by: Geliang Tang <geliangtang@gmail.com> --- fs/pstore/ram_core.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c index 76c3f80..a8500cd 100644 --- a/fs/pstore/ram_core.c +++ b/fs/pstore/ram_core.c @@ -54,7 +54,7 @@ static size_t buffer_start_add_atomic(struct persistent_ram_zone *prz, size_t a) int new; do { - old = atomic_read(&prz->buffer->start); + old = buffer_start(prz); new = old + a; while (unlikely(new >= prz->buffer_size)) new -= prz->buffer_size; @@ -69,11 +69,11 @@ static void buffer_size_add_atomic(struct persistent_ram_zone *prz, size_t a) size_t old; size_t new; - if (atomic_read(&prz->buffer->size) == prz->buffer_size) + if (buffer_size(prz) == prz->buffer_size) return; do { - old = atomic_read(&prz->buffer->size); + old = buffer_size(prz); new = old + a; if (new > prz->buffer_size) new = prz->buffer_size; @@ -91,7 +91,7 @@ static size_t buffer_start_add_locked(struct persistent_ram_zone *prz, size_t a) raw_spin_lock_irqsave(&buffer_lock, flags); - old = atomic_read(&prz->buffer->start); + old = buffer_start(prz); new = old + a; while (unlikely(new >= prz->buffer_size)) new -= prz->buffer_size; @@ -111,7 +111,7 @@ static void buffer_size_add_locked(struct persistent_ram_zone *prz, size_t a) raw_spin_lock_irqsave(&buffer_lock, flags); - old = atomic_read(&prz->buffer->size); + old = buffer_size(prz); if (old == prz->buffer_size) goto exit; -- 2.7.4 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 2/2] ramoops: use buffer_size() and buffer_start() 2016-08-30 12:24 ` [PATCH 2/2] ramoops: use buffer_size() and buffer_start() Geliang Tang @ 2016-09-08 20:44 ` Kees Cook 0 siblings, 0 replies; 4+ messages in thread From: Kees Cook @ 2016-09-08 20:44 UTC (permalink / raw) To: Geliang Tang; +Cc: Anton Vorontsov, Colin Cross, Tony Luck, LKML On Tue, Aug 30, 2016 at 5:24 AM, Geliang Tang <geliangtang@gmail.com> wrote: > Since buffer_size() and buffer_start() have been defined in ram_core.c, > use them instead of open-coding. > > Signed-off-by: Geliang Tang <geliangtang@gmail.com> Hi! Thanks for this clean-up. However, since the _atomic method has been removed entirely now, I don't want to make this change, since it makes reading the atomic_read/atomic_set in functions unmatched. I'd prefer to leave this as-is. -Kees > --- > fs/pstore/ram_core.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c > index 76c3f80..a8500cd 100644 > --- a/fs/pstore/ram_core.c > +++ b/fs/pstore/ram_core.c > @@ -54,7 +54,7 @@ static size_t buffer_start_add_atomic(struct persistent_ram_zone *prz, size_t a) > int new; > > do { > - old = atomic_read(&prz->buffer->start); > + old = buffer_start(prz); > new = old + a; > while (unlikely(new >= prz->buffer_size)) > new -= prz->buffer_size; > @@ -69,11 +69,11 @@ static void buffer_size_add_atomic(struct persistent_ram_zone *prz, size_t a) > size_t old; > size_t new; > > - if (atomic_read(&prz->buffer->size) == prz->buffer_size) > + if (buffer_size(prz) == prz->buffer_size) > return; > > do { > - old = atomic_read(&prz->buffer->size); > + old = buffer_size(prz); > new = old + a; > if (new > prz->buffer_size) > new = prz->buffer_size; > @@ -91,7 +91,7 @@ static size_t buffer_start_add_locked(struct persistent_ram_zone *prz, size_t a) > > raw_spin_lock_irqsave(&buffer_lock, flags); > > - old = atomic_read(&prz->buffer->start); > + old = buffer_start(prz); > new = old + a; > while (unlikely(new >= prz->buffer_size)) > new -= prz->buffer_size; > @@ -111,7 +111,7 @@ static void buffer_size_add_locked(struct persistent_ram_zone *prz, size_t a) > > raw_spin_lock_irqsave(&buffer_lock, flags); > > - old = atomic_read(&prz->buffer->size); > + old = buffer_size(prz); > if (old == prz->buffer_size) > goto exit; > > -- > 2.7.4 > -- Kees Cook Nexus Security ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] ramoops: move spin_lock_init after kmalloc error checking 2016-08-30 12:24 [PATCH 1/2] ramoops: move spin_lock_init after kmalloc error checking Geliang Tang 2016-08-30 12:24 ` [PATCH 2/2] ramoops: use buffer_size() and buffer_start() Geliang Tang @ 2016-09-08 20:45 ` Kees Cook 1 sibling, 0 replies; 4+ messages in thread From: Kees Cook @ 2016-09-08 20:45 UTC (permalink / raw) To: Geliang Tang; +Cc: Anton Vorontsov, Colin Cross, Tony Luck, LKML On Tue, Aug 30, 2016 at 5:24 AM, Geliang Tang <geliangtang@gmail.com> wrote: > If cxt->pstore.buf allocated failed, no need to initialize > cxt->pstore.buf_lock. So this patch moves spin_lock_init() after the > error checking. > > Signed-off-by: Geliang Tang <geliangtang@gmail.com> > --- > fs/pstore/ram.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c > index 7a034d6..ec1c9e5 100644 > --- a/fs/pstore/ram.c > +++ b/fs/pstore/ram.c > @@ -608,12 +608,12 @@ static int ramoops_probe(struct platform_device *pdev) > cxt->pstore.bufsize = 1024; /* LOG_LINE_MAX */ > cxt->pstore.bufsize = max(cxt->record_size, cxt->pstore.bufsize); > cxt->pstore.buf = kmalloc(cxt->pstore.bufsize, GFP_KERNEL); > - spin_lock_init(&cxt->pstore.buf_lock); > if (!cxt->pstore.buf) { > pr_err("cannot allocate pstore buffer\n"); > err = -ENOMEM; > goto fail_clear; > } > + spin_lock_init(&cxt->pstore.buf_lock); Seems fine to me. No harm either way, but better to save on the work. :) Applied for -next. Thanks! -Kees > > err = pstore_register(&cxt->pstore); > if (err) { > -- > 2.7.4 > -- Kees Cook Nexus Security ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-09-08 20:45 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-08-30 12:24 [PATCH 1/2] ramoops: move spin_lock_init after kmalloc error checking Geliang Tang 2016-08-30 12:24 ` [PATCH 2/2] ramoops: use buffer_size() and buffer_start() Geliang Tang 2016-09-08 20:44 ` Kees Cook 2016-09-08 20:45 ` [PATCH 1/2] ramoops: move spin_lock_init after kmalloc error checking Kees Cook
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).