All of lore.kernel.org
 help / color / mirror / Atom feed
From: puranjay12 at gmail.com (Puranjay Mohan)
Subject: [Linux-kernel-mentees] [PATCH 0/3] net: ethernet: atheros: atlx: Use PCI generic definitions instead of private duplicates
Date: Sat, 22 Jun 2019 14:20:01 +0530	[thread overview]
Message-ID: <20190622085001.GA11032@arch> (raw)
In-Reply-To: <838b8e84523151418ab8cda4abdbb114ce24a497.camel@perches.com>

On Fri, Jun 21, 2019 at 11:33:27AM -0700, Joe Perches wrote:
> On Fri, 2019-06-21 at 13:12 -0500, Bjorn Helgaas wrote:
> > On Fri, Jun 21, 2019 at 12:27 PM Joe Perches <joe at perches.com> wrote:
> []
> > > Subsystem specific local PCI #defines without generic
> > > naming is poor style and makes treewide grep and
> > > refactoring much more difficult.
> > 
> > Don't worry, we have the same objectives.  I totally agree that local
> > #defines are a bad thing, which is why I proposed this project in the
> > first place.
> 
> Hi again Bjorn.
> 
> I didn't know that was your idea.  Good idea.
> 
> > I'm just saying that this is a "first-patch" sort of learning project
> > and I think it'll avoid some list spamming and discouragement if we
> > can figure out the scope and shake out some of the teething problems
> > ahead of time.  I don't want to end up with multiple versions of
> > dozens of little 2-3 patch series posted every week or two.
> 
> Great, that's sensible.
> 
> > I'd rather be able to deal with a whole block of them at one time.
> 
> Also very sensible.
> 
> > > 2: Show that you compiled the object files and verified
> > >    where possible that there are no object file changes.
> > 
> > Do you have any pointers for the best way to do this?  Is it as simple
> > as comparing output of "objdump -d"?
> 
> Generically, yes.
> 
> I have a little script that does the equivalent of:
> 
> <git reset>
> make <foo.o>
> mv <foo.o> <foo.o>.old
> patch -P1 < <foo_patch>
> make <foo.o>
> mv <foo.o> <foo.o>.new
> diff -urN <(objdump -d <foo.o>.old) <(objdump -d <foo.o>.new)
> 
> But it's not foolproof as gcc does not guarantee
> compilation repeatability.
> 
> And some subsystems Makefiles do not allow per-file
> compilation.
>
Hi Joe,
I tried using your specified technique here are the steps I took and the
results I got.

I built the object file before the patch named it "atl2-old.o"
then I built it after the patch, named it "atl2-new.o"

then i ran these commands:-
$ objdump -d atl2-old.o > 1
$ objdump -d atl2-new.o > 2
$ diff -urN 1 2

--- 1	2019-06-22 13:56:17.881392372 +0530
+++ 2	2019-06-22 13:56:35.228018053 +0530
@@ -1,5 +1,5 @@

-atl2-old.o:     file format elf64-x86-64
+atl2-new.o:     file format elf64-x86-64


 Disassembly of section .text:

So both the object files are similar.

Thanks,
--Puranjay

WARNING: multiple messages have this Message-ID (diff)
From: puranjay12@gmail.com (Puranjay Mohan)
Subject: [Linux-kernel-mentees] [PATCH 0/3] net: ethernet: atheros: atlx: Use PCI generic definitions instead of private duplicates
Date: Sat, 22 Jun 2019 14:20:01 +0530	[thread overview]
Message-ID: <20190622085001.GA11032@arch> (raw)
Message-ID: <20190622085001.wlLXgPkkDgVruKZXagj1TjWzAiq97MXHl_NWPGY6P9M@z> (raw)
In-Reply-To: <838b8e84523151418ab8cda4abdbb114ce24a497.camel@perches.com>

On Fri, Jun 21, 2019 at 11:33:27AM -0700, Joe Perches wrote:
> On Fri, 2019-06-21 at 13:12 -0500, Bjorn Helgaas wrote:
> > On Fri, Jun 21, 2019 at 12:27 PM Joe Perches <joe at perches.com> wrote:
> []
> > > Subsystem specific local PCI #defines without generic
> > > naming is poor style and makes treewide grep and
> > > refactoring much more difficult.
> > 
> > Don't worry, we have the same objectives.  I totally agree that local
> > #defines are a bad thing, which is why I proposed this project in the
> > first place.
> 
> Hi again Bjorn.
> 
> I didn't know that was your idea.  Good idea.
> 
> > I'm just saying that this is a "first-patch" sort of learning project
> > and I think it'll avoid some list spamming and discouragement if we
> > can figure out the scope and shake out some of the teething problems
> > ahead of time.  I don't want to end up with multiple versions of
> > dozens of little 2-3 patch series posted every week or two.
> 
> Great, that's sensible.
> 
> > I'd rather be able to deal with a whole block of them at one time.
> 
> Also very sensible.
> 
> > > 2: Show that you compiled the object files and verified
> > >    where possible that there are no object file changes.
> > 
> > Do you have any pointers for the best way to do this?  Is it as simple
> > as comparing output of "objdump -d"?
> 
> Generically, yes.
> 
> I have a little script that does the equivalent of:
> 
> <git reset>
> make <foo.o>
> mv <foo.o> <foo.o>.old
> patch -P1 < <foo_patch>
> make <foo.o>
> mv <foo.o> <foo.o>.new
> diff -urN <(objdump -d <foo.o>.old) <(objdump -d <foo.o>.new)
> 
> But it's not foolproof as gcc does not guarantee
> compilation repeatability.
> 
> And some subsystems Makefiles do not allow per-file
> compilation.
>
Hi Joe,
I tried using your specified technique here are the steps I took and the
results I got.

I built the object file before the patch named it "atl2-old.o"
then I built it after the patch, named it "atl2-new.o"

then i ran these commands:-
$ objdump -d atl2-old.o > 1
$ objdump -d atl2-new.o > 2
$ diff -urN 1 2

--- 1	2019-06-22 13:56:17.881392372 +0530
+++ 2	2019-06-22 13:56:35.228018053 +0530
@@ -1,5 +1,5 @@

-atl2-old.o:     file format elf64-x86-64
+atl2-new.o:     file format elf64-x86-64


 Disassembly of section .text:

So both the object files are similar.

Thanks,
--Puranjay

WARNING: multiple messages have this Message-ID (diff)
From: Puranjay Mohan <puranjay12@gmail.com>
To: Joe Perches <joe@perches.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>,
	Puranjay Mohan <puranjay12@gmail.com>,
	Jay Cliburn <jcliburn@gmail.com>,
	Shuah Khan <skhan@linuxfoundation.org>,
	Bjorn Helgaas <bjorn@helgaas.com>,
	netdev <netdev@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-kernel-mentees@lists.linuxfoundation.org,
	Linux PCI <linux-pci@vger.kernel.org>,
	Chris Snook <chris.snook@gmail.com>
Subject: Re: [PATCH 0/3] net: ethernet: atheros: atlx: Use PCI generic definitions instead of private duplicates
Date: Sat, 22 Jun 2019 14:20:01 +0530	[thread overview]
Message-ID: <20190622085001.GA11032@arch> (raw)
In-Reply-To: <838b8e84523151418ab8cda4abdbb114ce24a497.camel@perches.com>

On Fri, Jun 21, 2019 at 11:33:27AM -0700, Joe Perches wrote:
> On Fri, 2019-06-21 at 13:12 -0500, Bjorn Helgaas wrote:
> > On Fri, Jun 21, 2019 at 12:27 PM Joe Perches <joe@perches.com> wrote:
> []
> > > Subsystem specific local PCI #defines without generic
> > > naming is poor style and makes treewide grep and
> > > refactoring much more difficult.
> > 
> > Don't worry, we have the same objectives.  I totally agree that local
> > #defines are a bad thing, which is why I proposed this project in the
> > first place.
> 
> Hi again Bjorn.
> 
> I didn't know that was your idea.  Good idea.
> 
> > I'm just saying that this is a "first-patch" sort of learning project
> > and I think it'll avoid some list spamming and discouragement if we
> > can figure out the scope and shake out some of the teething problems
> > ahead of time.  I don't want to end up with multiple versions of
> > dozens of little 2-3 patch series posted every week or two.
> 
> Great, that's sensible.
> 
> > I'd rather be able to deal with a whole block of them at one time.
> 
> Also very sensible.
> 
> > > 2: Show that you compiled the object files and verified
> > >    where possible that there are no object file changes.
> > 
> > Do you have any pointers for the best way to do this?  Is it as simple
> > as comparing output of "objdump -d"?
> 
> Generically, yes.
> 
> I have a little script that does the equivalent of:
> 
> <git reset>
> make <foo.o>
> mv <foo.o> <foo.o>.old
> patch -P1 < <foo_patch>
> make <foo.o>
> mv <foo.o> <foo.o>.new
> diff -urN <(objdump -d <foo.o>.old) <(objdump -d <foo.o>.new)
> 
> But it's not foolproof as gcc does not guarantee
> compilation repeatability.
> 
> And some subsystems Makefiles do not allow per-file
> compilation.
>
Hi Joe,
I tried using your specified technique here are the steps I took and the
results I got.

I built the object file before the patch named it "atl2-old.o"
then I built it after the patch, named it "atl2-new.o"

then i ran these commands:-
$ objdump -d atl2-old.o > 1
$ objdump -d atl2-new.o > 2
$ diff -urN 1 2

--- 1	2019-06-22 13:56:17.881392372 +0530
+++ 2	2019-06-22 13:56:35.228018053 +0530
@@ -1,5 +1,5 @@

-atl2-old.o:     file format elf64-x86-64
+atl2-new.o:     file format elf64-x86-64


 Disassembly of section .text:

So both the object files are similar.

Thanks,
--Puranjay




  parent reply	other threads:[~2019-06-22  8:50 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-21 16:39 [Linux-kernel-mentees] [PATCH 0/3] net: ethernet: atheros: atlx: Use PCI generic definitions instead of private duplicates puranjay12
2019-06-21 16:39 ` Puranjay Mohan
2019-06-21 16:39 ` [Linux-kernel-mentees] " Puranjay Mohan
2019-06-21 16:39 ` [Linux-kernel-mentees] [PATCH 1/3] net: ethernet: atheros: atlx: Rename local PCI defines to generic names puranjay12
2019-06-21 16:39   ` Puranjay Mohan
2019-06-21 16:39   ` [Linux-kernel-mentees] " Puranjay Mohan
2019-06-21 16:39 ` [Linux-kernel-mentees] [PATCH 2/3] net: ethernet: atheros: atlx: Include generic PCI definitions puranjay12
2019-06-21 16:39   ` Puranjay Mohan
2019-06-21 16:39   ` [Linux-kernel-mentees] " Puranjay Mohan
2019-06-21 16:39 ` [Linux-kernel-mentees] [PATCH 3/3] net: ethernet: atheros: atlx: Remove unused and private " puranjay12
2019-06-21 16:39   ` Puranjay Mohan
2019-06-21 16:39   ` [Linux-kernel-mentees] " Puranjay Mohan
2019-06-21 17:13   ` bhelgaas
2019-06-21 17:13     ` Bjorn Helgaas
2019-06-21 17:13     ` [Linux-kernel-mentees] " Bjorn Helgaas
2019-06-26 20:07   ` davem
2019-06-26 20:07     ` David Miller
2019-06-26 20:07     ` [Linux-kernel-mentees] " David Miller
2019-06-21 17:11 ` [Linux-kernel-mentees] [PATCH 0/3] net: ethernet: atheros: atlx: Use PCI generic definitions instead of private duplicates bhelgaas
2019-06-21 17:11   ` Bjorn Helgaas
2019-06-21 17:11   ` [Linux-kernel-mentees] " Bjorn Helgaas
2019-06-21 17:27   ` joe
2019-06-21 17:27     ` Joe Perches
2019-06-21 17:27     ` [Linux-kernel-mentees] " Joe Perches
2019-06-21 18:12     ` bhelgaas
2019-06-21 18:12       ` Bjorn Helgaas
2019-06-21 18:12       ` [Linux-kernel-mentees] " Bjorn Helgaas
2019-06-21 18:33       ` joe
2019-06-21 18:33         ` Joe Perches
2019-06-21 18:33         ` [Linux-kernel-mentees] " Joe Perches
2019-06-21 22:45         ` chris.snook
2019-06-21 22:45           ` Chris Snook
2019-06-21 22:45           ` [Linux-kernel-mentees] " Chris Snook
2019-06-22  8:50         ` puranjay12 [this message]
2019-06-22  8:50           ` Puranjay Mohan
2019-06-22  8:50           ` [Linux-kernel-mentees] " Puranjay Mohan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190622085001.GA11032@arch \
    --to=unknown@example.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.