lkml.org 
[lkml]   [2012]   [Apr]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH 6/6] tilegx network driver: initial support
    Date
    On Thursday 12 April 2012, Chris Metcalf wrote:
    > On 4/10/2012 6:42 AM, Arnd Bergmann wrote:
    > > Ok, but please remove tile_net_devs then.
    > > I think a better abstraction for tile_net_devs_for_channel would be
    > > some interface that lets you add private data to a channel so when
    > > you get data from a channel, you can extract that pointer from the driver
    > > using the channel.

    > I think what would be clearer is to document how and why we are using this
    > additional data structure. We do access via both arrays where it is
    > efficient to do so, so getting rid of either of them doesn't seem right.
    > Let's keep the "normal" tile_net_devs[] as is, indexed by devno, and make
    > the tile_net_devs_for_channel[] more abstracted by using the following code:

    The tile_net_devs still feels dirty. You basically only
    use it in tile_net_handle_egress_timer(), but there you don't
    actually take the mutex that protects addition and removal from
    the array, so it's racy in case of hotplug.

    A more conservative way to do this is to have the timer per
    device (or by channel, if you like), so it does not have to
    iterate the array.

    > /*
    > * Provide support for efficiently mapping a channel to the device
    > * that is using that channel, or NULL if none. The pointers in this
    > * array are only non-NULL when pointing to active tilegx net_device
    > * structures, and they are cleared before the struture itself is
    > * released.
    > *
    > * HACK: We use "32", not TILE_NET_CHANNELS, because it is fairly
    > * subtle that the 5 bit "idesc.channel" field never exceeds
    > * TILE_NET_CHANNELS.
    > */
    > static struct net_device *channel_to_dev[32];
    >
    > static void bychannel_add(struct net_device *dev)
    > {
    > struct tile_net_priv *priv = netdev_priv(dev);
    > BUG_ON(channel_to_dev[priv->channel] != NULL);
    > channel_to_dev[priv->channel] = dev;
    > }
    >
    > static void bychannel_delete(struct net_device *dev)
    > {
    > struct tile_net_priv *priv = netdev_priv(dev);
    > channel_to_dev[priv->channel] = NULL;
    > }
    >
    > static inline struct net_device *bychannel_lookup(int channel)
    > {
    > return channel_to_dev[channel];
    > }
    >
    >
    > We then call bychannel_add() in the open method, and bychannel_delete() in
    > the close method, so it's clear that the pointers have appropriate lifetimes.

    Ok.

    Arnd


    \
     
     \ /
      Last update: 2012-04-13 12:37    [W:0.025 / U:150.304 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site