* [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).