[v3,1/6] android: binder: Refactor prev and next buffer into a helper function
diff mbox series

Message ID 20170830004702.120371-2-sherryy@android.com
State New, archived
Headers show
Series
  • android: binder: move allocator metadata and add shrinker
Related show

Commit Message

Sherry Yang Aug. 30, 2017, 12:46 a.m. UTC
Use helper functions buffer_next and buffer_prev instead
of list_entry to get the next and previous buffers.

Signed-off-by: Sherry Yang <sherryy@android.com>
---
 drivers/android/binder_alloc.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

Comments

Greg KH Aug. 30, 2017, 6:07 a.m. UTC | #1
On Tue, Aug 29, 2017 at 05:46:57PM -0700, Sherry Yang wrote:
> Use helper functions buffer_next and buffer_prev instead
> of list_entry to get the next and previous buffers.
> 
> Signed-off-by: Sherry Yang <sherryy@android.com>
> ---
>  drivers/android/binder_alloc.c | 24 +++++++++++++++---------
>  1 file changed, 15 insertions(+), 9 deletions(-)

What changed from the previous version?

Always put the changes below the --- line.  Like the documentation says
to do so.

Also, don't I already have these patches in my tree?  Do you want me to
revert the existing ones and use the new ones, or what about a fixup
patch for any differences?

confused,

greg k-h
Sherry Yang Aug. 30, 2017, 7:46 p.m. UTC | #2
On Tue, Aug 29, 2017 at 11:07 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Tue, Aug 29, 2017 at 05:46:57PM -0700, Sherry Yang wrote:
>> Use helper functions buffer_next and buffer_prev instead
>> of list_entry to get the next and previous buffers.
>>
>> Signed-off-by: Sherry Yang <sherryy@android.com>
>> ---
>>  drivers/android/binder_alloc.c | 24 +++++++++++++++---------
>>  1 file changed, 15 insertions(+), 9 deletions(-)
>
> What changed from the previous version?

This specific patch didn't change. The change is only in [patch v3
3/6] (crash fix) and in [patch v3 6/6] (new patch that add stats).

> Always put the changes below the --- line.  Like the documentation says
> to do so.

Do you mean I should use --annotate to git send-email to specify what
has changed across versions for each patch? I used --compose and put
what changed from v2 to v3 in the introductory message [patch 0/6].

> Also, don't I already have these patches in my tree?  Do you want me to
> revert the existing ones and use the new ones, or what about a fixup
> patch for any differences?

I got a message saying a test failed on [patch 3/5]. I resubmitted the
entire thread because I wasn't sure if you would drop the failing
patch set. If the entire failing patch set is dropped, then v3 can be
used as a replacement.

If you prefer a fixup patch, I can post another patch set (the crash
fix and the new patch) on top of what you already applied, but it
might be easier to do what's described in the previous paragraph (drop
v2 and apply v3).

Sherry Yang

> confused,
>
> greg k-h
Dan Carpenter Aug. 30, 2017, 8:08 p.m. UTC | #3
On Wed, Aug 30, 2017 at 12:46:38PM -0700, Sherry Yang wrote:
> I used --compose and put what changed from v2 to v3 in the
> introductory message [patch 0/6].

Hm...  I never got [patch 0/6] (I'm on devel@driverdev.osuosl.org).

regards,
dan carpenter
Greg KH Aug. 31, 2017, 4:28 a.m. UTC | #4
On Wed, Aug 30, 2017 at 12:46:38PM -0700, Sherry Yang wrote:
> On Tue, Aug 29, 2017 at 11:07 PM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > On Tue, Aug 29, 2017 at 05:46:57PM -0700, Sherry Yang wrote:
> >> Use helper functions buffer_next and buffer_prev instead
> >> of list_entry to get the next and previous buffers.
> >>
> >> Signed-off-by: Sherry Yang <sherryy@android.com>
> >> ---
> >>  drivers/android/binder_alloc.c | 24 +++++++++++++++---------
> >>  1 file changed, 15 insertions(+), 9 deletions(-)
> >
> > What changed from the previous version?
> 
> This specific patch didn't change. The change is only in [patch v3
> 3/6] (crash fix) and in [patch v3 6/6] (new patch that add stats).

I need to know that :)

> > Always put the changes below the --- line.  Like the documentation says
> > to do so.
> 
> Do you mean I should use --annotate to git send-email to specify what
> has changed across versions for each patch? I used --compose and put
> what changed from v2 to v3 in the introductory message [patch 0/6].

I never got a patch 0/6 here, and honestly, almost always ignore them as
patches should be self-obvious :)

> > Also, don't I already have these patches in my tree?  Do you want me to
> > revert the existing ones and use the new ones, or what about a fixup
> > patch for any differences?
> 
> I got a message saying a test failed on [patch 3/5]. I resubmitted the
> entire thread because I wasn't sure if you would drop the failing
> patch set. If the entire failing patch set is dropped, then v3 can be
> used as a replacement.

Do you want me to revert all of these?  I can not "drop" them as my tree
can not rebase.  If it's just a simple fix, I'll gladly take that
instead.

> If you prefer a fixup patch, I can post another patch set (the crash
> fix and the new patch) on top of what you already applied, but it
> might be easier to do what's described in the previous paragraph (drop
> v2 and apply v3).

Ok, exactly what git commit ids should I revert from the tree?  But
really, a fix would be easier at this point :)

thanks,

greg k-h
Sherry Yang Aug. 31, 2017, 5:30 p.m. UTC | #5
On Wed, Aug 30, 2017 at 9:28 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> Ok, exactly what git commit ids should I revert from the tree?  But
> really, a fix would be easier at this point :)

I sent a fixup patch as a separate reply to this email. Please also
apply [patch v3 6/6] as it was not in v2, which you applied.

Thanks,
Sherry
Greg KH Aug. 31, 2017, 6:47 p.m. UTC | #6
On Thu, Aug 31, 2017 at 10:30:27AM -0700, Sherry Yang wrote:
> On Wed, Aug 30, 2017 at 9:28 PM, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > Ok, exactly what git commit ids should I revert from the tree?  But
> > really, a fix would be easier at this point :)
> 
> I sent a fixup patch as a separate reply to this email. Please also
> apply [patch v3 6/6] as it was not in v2, which you applied.

Please resend that patch, it's no longer in my queue.

thanks,

greg k-h

Patch
diff mbox series

diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index 40f31df60580..f15af2b55a62 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -48,14 +48,23 @@  module_param_named(debug_mask, binder_alloc_debug_mask,
 			pr_info(x); \
 	} while (0)
 
+static struct binder_buffer *binder_buffer_next(struct binder_buffer *buffer)
+{
+	return list_entry(buffer->entry.next, struct binder_buffer, entry);
+}
+
+static struct binder_buffer *binder_buffer_prev(struct binder_buffer *buffer)
+{
+	return list_entry(buffer->entry.prev, struct binder_buffer, entry);
+}
+
 static size_t binder_alloc_buffer_size(struct binder_alloc *alloc,
 				       struct binder_buffer *buffer)
 {
 	if (list_is_last(&buffer->entry, &alloc->buffers))
 		return alloc->buffer +
 		       alloc->buffer_size - (void *)buffer->data;
-	return (size_t)list_entry(buffer->entry.next,
-			  struct binder_buffer, entry) - (size_t)buffer->data;
+	return (size_t)binder_buffer_next(buffer) - (size_t)buffer->data;
 }
 
 static void binder_insert_free_buffer(struct binder_alloc *alloc,
@@ -470,7 +479,7 @@  static void binder_delete_free_buffer(struct binder_alloc *alloc,
 	int free_page_start = 1;
 
 	BUG_ON(alloc->buffers.next == &buffer->entry);
-	prev = list_entry(buffer->entry.prev, struct binder_buffer, entry);
+	prev = binder_buffer_prev(buffer);
 	BUG_ON(!prev->free);
 	if (buffer_end_page(prev) == buffer_start_page(buffer)) {
 		free_page_start = 0;
@@ -482,8 +491,7 @@  static void binder_delete_free_buffer(struct binder_alloc *alloc,
 	}
 
 	if (!list_is_last(&buffer->entry, &alloc->buffers)) {
-		next = list_entry(buffer->entry.next,
-				  struct binder_buffer, entry);
+		next = binder_buffer_next(buffer);
 		if (buffer_start_page(next) == buffer_end_page(buffer)) {
 			free_page_end = 0;
 			if (buffer_start_page(next) ==
@@ -544,8 +552,7 @@  static void binder_free_buf_locked(struct binder_alloc *alloc,
 	rb_erase(&buffer->rb_node, &alloc->allocated_buffers);
 	buffer->free = 1;
 	if (!list_is_last(&buffer->entry, &alloc->buffers)) {
-		struct binder_buffer *next = list_entry(buffer->entry.next,
-						struct binder_buffer, entry);
+		struct binder_buffer *next = binder_buffer_next(buffer);
 
 		if (next->free) {
 			rb_erase(&next->rb_node, &alloc->free_buffers);
@@ -553,8 +560,7 @@  static void binder_free_buf_locked(struct binder_alloc *alloc,
 		}
 	}
 	if (alloc->buffers.next != &buffer->entry) {
-		struct binder_buffer *prev = list_entry(buffer->entry.prev,
-						struct binder_buffer, entry);
+		struct binder_buffer *prev = binder_buffer_prev(buffer);
 
 		if (prev->free) {
 			binder_delete_free_buffer(alloc, buffer);