The dls_devnet_create() function enters the MAC perimeter:
int
dls_devnet_create(mac_handle_t mh, datalink_id_t linkid)
{
dls_link_t *dlp;
int err;
mac_perim_handle_t mph;
mac_perim_enter_by_mh(mh, &mph);
...
if ((err = dls_devnet_set(mac_name(mh), linkid, NULL)) != 0) {
...
if ((err = dls_link_hold_create(mac_name(mh), &dlp)) != 0) {
...
mac_perim_exit(mph);
return (err);
}
None of the above constitute direct control operations on the MAC, so
why is the perimeter entered? Does calling mac_name() really require
entering the perimeter?
-Seb
Sebastien Roy wrote:> The dls_devnet_create() function enters the MAC perimeter: > > int > dls_devnet_create(mac_handle_t mh, datalink_id_t linkid) > { > dls_link_t *dlp; > int err; > mac_perim_handle_t mph; > > mac_perim_enter_by_mh(mh, &mph); > ... > if ((err = dls_devnet_set(mac_name(mh), linkid, NULL)) != 0) { > ... > if ((err = dls_link_hold_create(mac_name(mh), &dlp)) != 0) { > ... > mac_perim_exit(mph); > return (err); > } > > None of the above constitute direct control operations on the MAC, so > why is the perimeter entered? Does calling mac_name() really require > entering the perimeter? >It is needed for the dls_link_hold_create(), which may actually end up doing a mac_open and mac_client_open. It is also useful in serializing the dls_devnet_set() and a concurrent dls_devnet_rename(). Thirumalai> -Seb > > > _______________________________________________ > crossbow-discuss mailing list > crossbow-discuss at opensolaris.org > http://mail.opensolaris.org/mailman/listinfo/crossbow-discuss >
On Fri, 2009-02-13 at 11:25 -0800, Thirumalai Srinivasan wrote:> Sebastien Roy wrote: > > The dls_devnet_create() function enters the MAC perimeter: > > > > int > > dls_devnet_create(mac_handle_t mh, datalink_id_t linkid) > > { > > dls_link_t *dlp; > > int err; > > mac_perim_handle_t mph; > > > > mac_perim_enter_by_mh(mh, &mph); > > ... > > if ((err = dls_devnet_set(mac_name(mh), linkid, NULL)) != 0) { > > ... > > if ((err = dls_link_hold_create(mac_name(mh), &dlp)) != 0) { > > ... > > mac_perim_exit(mph); > > return (err); > > } > > > > None of the above constitute direct control operations on the MAC, so > > why is the perimeter entered? Does calling mac_name() really require > > entering the perimeter? > > > It is needed for the dls_link_hold_create(), which may actually end up > doing a mac_open and mac_client_open. It is also useful in serializing > the dls_devnet_set() and a concurrent dls_devnet_rename().I''m sorry, I''m still a bit lost; Why would holding the perimeter be required to call mac_open() or mac_client_open()? Regarding the usefulness of the perimeter as a synchronization of dls_devnet_*() operations, i_dls_devnet_lock already serializes these operations, but the comment in dls_devnet_rename() hints at some relationship to property loading: /* * The framework does not hold hold locks across calls to the * mac perimeter, hence enter the perimeter first. This also waits * for the property loading to finish. */ if ((err = mac_perim_enter_by_linkid(id1, &mph)) != 0) goto done; Couldn''t dls_devnet_prop_task_wait() be used to wait for property loading to finish? Note that I don''t have a specific problem with the way the MAC perimeters are being used here, I''m just trying to understand the code. -Seb
Sebastien Roy wrote:> >> It is needed for the dls_link_hold_create(), which may actually end up >> doing a mac_open and mac_client_open. It is also useful in serializing >> the dls_devnet_set() and a concurrent dls_devnet_rename(). >> > > I''m sorry, I''m still a bit lost; Why would holding the perimeter be > required to call mac_open() or mac_client_open()? > >ok. Here is a more detailed explanation. The creation of a dls_link_t involves multiple operations * Allocation of the dls_link_t * Initializing fields of the dls_link_t. Eg. dl_mh from the mac_open and dl_mch from the mac_client_open * Insertion of the dls_link_t in the i_dls_link_hash modhash list. Now consider a lookup of a dls_link_t. For example the sequence in dld_open -> dld_str_create -> dls_link_hold. This would just lookup the i_dls_link_hash modhash and return the dls_link_t. The mac perimeter makes the entire creation operation with its multiple sub steps atomic with respect to the lookup and prevents the lookup from seeing partly initialized mutually inconsistent fields.> Regarding the usefulness of the perimeter as a synchronization of > dls_devnet_*() operations, i_dls_devnet_lock already serializes these > operations, but the comment in dls_devnet_rename() hints at some > relationship to property loading: > > /* > * The framework does not hold hold locks across calls to the > * mac perimeter, hence enter the perimeter first. This also waits > * for the property loading to finish. > */ > if ((err = mac_perim_enter_by_linkid(id1, &mph)) != 0) > goto done; > > Couldn''t dls_devnet_prop_task_wait() be used to wait for property > loading to finish? > >This is not directly related to the property taskq thread. Both creation and rename are multi-step operations. The perimeter is a big lock that serializes creation, lookup, rename, destroy, or rather any multi-step control operation on the link. The property taskq loading proceeds independently and any operation that needs to wait for that can use dls_devnet_prop_task_wait. But the property taskq wait has other problems. (6738489) Thirumalai> Note that I don''t have a specific problem with the way the MAC > perimeters are being used here, I''m just trying to understand the code. > >> -Seb > > >
On Fri, 2009-02-13 at 13:14 -0800, Thirumalai Srinivasan wrote:> Sebastien Roy wrote: > > > >> It is needed for the dls_link_hold_create(), which may actually end up > >> doing a mac_open and mac_client_open. It is also useful in serializing > >> the dls_devnet_set() and a concurrent dls_devnet_rename(). > >> > > > > I''m sorry, I''m still a bit lost; Why would holding the perimeter be > > required to call mac_open() or mac_client_open()? > > > > > ok. Here is a more detailed explanation. > The creation of a dls_link_t involves multiple operations > > * Allocation of the dls_link_t > * Initializing fields of the dls_link_t. Eg. dl_mh from the mac_open and > dl_mch from the mac_client_open > * Insertion of the dls_link_t in the i_dls_link_hash modhash list. > > Now consider a lookup of a dls_link_t. For example the sequence in > dld_open -> dld_str_create -> dls_link_hold. This would just lookup the > i_dls_link_hash modhash and return the dls_link_t. > > The mac perimeter makes the entire creation operation with its multiple > sub steps atomic with respect to the lookup and prevents the lookup from > seeing partly initialized mutually inconsistent fields.Okay, thanks.> > > Regarding the usefulness of the perimeter as a synchronization of > > dls_devnet_*() operations, i_dls_devnet_lock already serializes these > > operations, but the comment in dls_devnet_rename() hints at some > > relationship to property loading: > > > > /* > > * The framework does not hold hold locks across calls to the > > * mac perimeter, hence enter the perimeter first. This also waits > > * for the property loading to finish. > > */ > > if ((err = mac_perim_enter_by_linkid(id1, &mph)) != 0) > > goto done; > > > > Couldn''t dls_devnet_prop_task_wait() be used to wait for property > > loading to finish? > > > > > This is not directly related to the property taskq thread. Both > creation and rename are multi-step operations. The perimeter is a big > lock that serializes creation, lookup, rename, destroy, or rather any > multi-step control operation on the link. The property taskq loading > proceeds independently and any operation that needs to wait for that can > use dls_devnet_prop_task_wait. But the property taskq wait has other > problems. (6738489)Got it. Thanks. -Seb