Messages in this thread | | | Subject | RE: [PATCH v2 03/10] RapidIO: Use stored ingress port number instead of register read | Date | Mon, 20 Sep 2010 07:31:22 -0700 | From | "Bounine, Alexandre" <> |
| |
Andrew Morton <akpm@linux-foundation.org> wrote: > > > @@ -219,6 +219,7 @@ struct rio_net { > > /** > > * struct rio_switch - RIO switch info > > * @node: Node in global list of switches > > + * @rdev: Associated RIO device structure > > * @switchid: Switch ID that is unique across a network > > * @hopcount: Hopcount to this switch > > * @destid: Associated destid in the path > > @@ -234,6 +235,7 @@ struct rio_net { > > */ > > struct rio_switch { > > struct list_head node; > > + struct rio_dev *rdev; > > u16 switchid; > > u16 hopcount; > > u16 destid; > > What is the locking for rdev?
This question prompted me consider the following change:
Because the rio_switch structure (in current implementation) is a dynamically allocated part of rio_dev, I think it may be simpler to combine them into single allocation by using zero length array declaration:
struct rio_dev { struct list_head global_list; struct list_head net_list; ..... ..... rest of rio_dev ..... struct rio_switch switch[0]; }
This will remove extra memory allocation, remove overlapping structure members and clean code sections like one shown below:
u8 hopcount = 0xff; u16 destid = rdev->destid;
if (rdev->rswitch) { destid = rdev->rswitch->destid; hopcount = rdev->rswitch->hopcount; }
And this looks better aligned with RapidIO definitions - both: endpoints and switches are RIO devices. The current implementation of rio_dev somewhat separates rio_switch from its common part (this is why I have added that link into rio_switch). We may keep using the pointer to associated rio_dev, but reworking the rio_dev structure seems better way to me.
Please, let me know what do you think about this conversion. And if there are no objections I will make a patch.
Alex.
| |