All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
To: Dave Young <dyoung@redhat.com>
Cc: kexec@lists.infradead.org, linuxppc-dev@lists.ozlabs.org,
	linux-kernel@vger.kernel.org,
	Eric Biederman <ebiederm@xmission.com>
Subject: Re: [PATCH v3 2/9] kexec_file: Generalize kexec_add_buffer.
Date: Thu, 30 Jun 2016 13:42:25 -0300	[thread overview]
Message-ID: <1902156.s2yTykCR7c@hactar> (raw)
In-Reply-To: <1866019.1p7Z0CLUZh@hactar>

Am Donnerstag, 30 Juni 2016, 12:49:44 schrieb Thiago Jung Bauermann:
> Am Donnerstag, 30 Juni 2016, 11:07:00 schrieb Dave Young:
> > On 06/29/16 at 06:18pm, Thiago Jung Bauermann wrote:
> > > I'm not following. The IMA buffer patchset doesn't use
> > > kexec_locate_mem_hole nor struct kexec_buf.
> > 
> > It does not use kexec_locate_mem_hole, but the buffer being passed is
> > very similar to a kexec_buf struct, no?
> 
> If what you're saying is that the arguments passed to
> kexec_add_handover_buffer in the IMA buffer patchset are very similar to
> the arguments passed to kexec_add_buffer then yes, it's true.
> 
> > So you may refactor kexec_add_buffer and your new function to pass only
> > kimage and a kbuf, it will be better than passing all those arguments
> > separately.
> 
> To be honest I think struct kexec_buf is an implementation detail inside
> kexec_locate_mem_hole, made necessary because the callback functions it
> uses need to access its arguments. Callers of kexec_locate_mem_hole,
> kexec_add_handover_buffer and kexec_add_buffer shouldn't need to know it
> exists.

Elaborating a bit more: the argument list for these three functions are 
equal or similar because kexec_add_handover_buffer uses kexec_add_buffer, 
which uses kexec_locate_mem_hole.

It could be beneficial to have a struct to collect the arguments to these 
functions if someone using one of them would be likely to use another one 
with the same arguments. In that case, you set up kexec_buf once and then 
just pass it whenever you need to call one of those functions.

But that is unlikely to happen. A user of the kexec API will need to use 
only one of these functions with a given set of arguments, so they don't 
gain anything by setting up a struct.

Syntactically, I also don't think it's clearer to set struct members instead 
of simply passing arguments to a function, even if the argument list is 
long.

[]'s
Thiago Jung Bauermann
IBM Linux Technology Center

WARNING: multiple messages have this Message-ID (diff)
From: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
To: Dave Young <dyoung@redhat.com>
Cc: linuxppc-dev@lists.ozlabs.org, kexec@lists.infradead.org,
	linux-kernel@vger.kernel.org,
	Eric Biederman <ebiederm@xmission.com>
Subject: Re: [PATCH v3 2/9] kexec_file: Generalize kexec_add_buffer.
Date: Thu, 30 Jun 2016 13:42:25 -0300	[thread overview]
Message-ID: <1902156.s2yTykCR7c@hactar> (raw)
In-Reply-To: <1866019.1p7Z0CLUZh@hactar>

Am Donnerstag, 30 Juni 2016, 12:49:44 schrieb Thiago Jung Bauermann:
> Am Donnerstag, 30 Juni 2016, 11:07:00 schrieb Dave Young:
> > On 06/29/16 at 06:18pm, Thiago Jung Bauermann wrote:
> > > I'm not following. The IMA buffer patchset doesn't use
> > > kexec_locate_mem_hole nor struct kexec_buf.
> > 
> > It does not use kexec_locate_mem_hole, but the buffer being passed is
> > very similar to a kexec_buf struct, no?
> 
> If what you're saying is that the arguments passed to
> kexec_add_handover_buffer in the IMA buffer patchset are very similar to
> the arguments passed to kexec_add_buffer then yes, it's true.
> 
> > So you may refactor kexec_add_buffer and your new function to pass only
> > kimage and a kbuf, it will be better than passing all those arguments
> > separately.
> 
> To be honest I think struct kexec_buf is an implementation detail inside
> kexec_locate_mem_hole, made necessary because the callback functions it
> uses need to access its arguments. Callers of kexec_locate_mem_hole,
> kexec_add_handover_buffer and kexec_add_buffer shouldn't need to know it
> exists.

Elaborating a bit more: the argument list for these three functions are 
equal or similar because kexec_add_handover_buffer uses kexec_add_buffer, 
which uses kexec_locate_mem_hole.

It could be beneficial to have a struct to collect the arguments to these 
functions if someone using one of them would be likely to use another one 
with the same arguments. In that case, you set up kexec_buf once and then 
just pass it whenever you need to call one of those functions.

But that is unlikely to happen. A user of the kexec API will need to use 
only one of these functions with a given set of arguments, so they don't 
gain anything by setting up a struct.

Syntactically, I also don't think it's clearer to set struct members instead 
of simply passing arguments to a function, even if the argument list is 
long.

[]'s
Thiago Jung Bauermann
IBM Linux Technology Center


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

  reply	other threads:[~2016-06-30 19:12 UTC|newest]

Thread overview: 94+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-21 19:48 [PATCH v3 0/9] kexec_file_load implementation for PowerPC Thiago Jung Bauermann
2016-06-21 19:48 ` Thiago Jung Bauermann
2016-06-21 19:48 ` [PATCH v3 1/9] kexec_file: Remove unused members from struct kexec_buf Thiago Jung Bauermann
2016-06-21 19:48   ` Thiago Jung Bauermann
2016-06-21 19:48 ` [PATCH v3 2/9] kexec_file: Generalize kexec_add_buffer Thiago Jung Bauermann
2016-06-21 19:48   ` Thiago Jung Bauermann
2016-06-22 10:20   ` Dave Young
2016-06-22 10:20     ` Dave Young
2016-06-22 23:30     ` Thiago Jung Bauermann
2016-06-22 23:30       ` Thiago Jung Bauermann
2016-06-23  2:25       ` Dave Young
2016-06-23  2:25         ` Dave Young
2016-06-28 22:18         ` Thiago Jung Bauermann
2016-06-28 22:18           ` Thiago Jung Bauermann
2016-06-29 19:47           ` Dave Young
2016-06-29 19:47             ` Dave Young
2016-06-29 21:18             ` Thiago Jung Bauermann
2016-06-29 21:18               ` Thiago Jung Bauermann
2016-06-30 15:07               ` Dave Young
2016-06-30 15:07                 ` Dave Young
2016-06-30 15:49                 ` Thiago Jung Bauermann
2016-06-30 15:49                   ` Thiago Jung Bauermann
2016-06-30 16:42                   ` Thiago Jung Bauermann [this message]
2016-06-30 16:42                     ` Thiago Jung Bauermann
2016-06-30 21:43                     ` Dave Young
2016-06-30 21:43                       ` Dave Young
2016-07-01 17:51                       ` Thiago Jung Bauermann
2016-07-01 17:51                         ` Thiago Jung Bauermann
2016-07-01 18:36                         ` Dave Young
2016-07-01 18:36                           ` Dave Young
2016-07-01 20:02                           ` Thiago Jung Bauermann
2016-07-01 20:02                             ` Thiago Jung Bauermann
2016-07-01 20:31                             ` Thiago Jung Bauermann
2016-07-01 20:31                               ` Thiago Jung Bauermann
2016-07-05  0:55                               ` Dave Young
2016-07-05  0:55                                 ` Dave Young
2016-06-21 19:48 ` [PATCH v3 3/9] kexec_file: Factor out kexec_locate_mem_hole from kexec_add_buffer Thiago Jung Bauermann
2016-06-21 19:48   ` Thiago Jung Bauermann
2016-06-22 10:18   ` Dave Young
2016-06-22 10:18     ` Dave Young
2016-06-22 23:34     ` Thiago Jung Bauermann
2016-06-22 23:34       ` Thiago Jung Bauermann
2016-06-23  2:30       ` Dave Young
2016-06-23  2:30         ` Dave Young
2016-06-23  5:44         ` Dave Young
2016-06-23  5:44           ` Dave Young
2016-06-23 15:37           ` Thiago Jung Bauermann
2016-06-23 15:37             ` Thiago Jung Bauermann
2016-06-27 16:19             ` Dave Young
2016-06-27 16:19               ` Dave Young
2016-06-27 16:37               ` Thiago Jung Bauermann
2016-06-27 16:37                 ` Thiago Jung Bauermann
2016-06-27 16:51                 ` Thiago Jung Bauermann
2016-06-27 16:51                   ` Thiago Jung Bauermann
2016-06-27 20:21                 ` Dave Young
2016-06-27 20:21                   ` Dave Young
2016-06-28 19:20                   ` Dave Young
2016-06-28 19:20                     ` Dave Young
2016-06-28 22:18                     ` Thiago Jung Bauermann
2016-06-28 22:18                       ` Thiago Jung Bauermann
2016-06-29 19:45                       ` Dave Young
2016-06-29 19:45                         ` Dave Young
2016-06-29 21:09                         ` Thiago Jung Bauermann
2016-06-29 21:09                           ` Thiago Jung Bauermann
2016-06-30 15:41                           ` Dave Young
2016-06-30 15:41                             ` Dave Young
2016-06-30 16:08                             ` Thiago Jung Bauermann
2016-06-30 16:08                               ` Thiago Jung Bauermann
2016-06-30 21:37                               ` Dave Young
2016-06-30 21:37                                 ` Dave Young
2016-06-21 19:48 ` [PATCH v3 4/9] powerpc: Factor out relocation code from module_64.c to elf_util_64.c Thiago Jung Bauermann
2016-06-21 19:48   ` Thiago Jung Bauermann
2016-06-21 19:48 ` [PATCH v3 5/9] powerpc: Generalize elf64_apply_relocate_add Thiago Jung Bauermann
2016-06-21 19:48   ` Thiago Jung Bauermann
2016-06-21 19:48 ` [PATCH v3 6/9] powerpc: Add functions to read ELF files of any endianness Thiago Jung Bauermann
2016-06-21 19:48   ` Thiago Jung Bauermann
2016-06-21 19:48 ` [PATCH v3 7/9] powerpc: Implement kexec_file_load Thiago Jung Bauermann
2016-06-21 19:48   ` Thiago Jung Bauermann
2016-06-21 19:48 ` [PATCH v3 8/9] powerpc: Add support for loading ELF kernels with kexec_file_load Thiago Jung Bauermann
2016-06-21 19:48   ` Thiago Jung Bauermann
2016-06-21 19:48 ` [PATCH v3 9/9] powerpc: Add purgatory for kexec_file_load implementation Thiago Jung Bauermann
2016-06-21 19:48   ` Thiago Jung Bauermann
2016-06-22 13:29 ` [PATCH v3 0/9] kexec_file_load implementation for PowerPC Balbir Singh
2016-06-22 13:29   ` Balbir Singh
2016-06-22 17:02   ` Thiago Jung Bauermann
2016-06-22 17:02     ` Thiago Jung Bauermann
2016-06-22 23:57     ` Balbir Singh
2016-06-22 23:57       ` Balbir Singh
2016-06-23 16:44       ` Thiago Jung Bauermann
2016-06-23 16:44         ` Thiago Jung Bauermann
2016-06-23 22:33         ` Balbir Singh
2016-06-23 22:33           ` Balbir Singh
2016-06-23 23:49           ` Thiago Jung Bauermann
2016-06-23 23:49             ` Thiago Jung Bauermann

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=1902156.s2yTykCR7c@hactar \
    --to=bauerman@linux.vnet.ibm.com \
    --cc=dyoung@redhat.com \
    --cc=ebiederm@xmission.com \
    --cc=kexec@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    /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.